-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add JSON Page #7
base: master
Are you sure you want to change the base?
Conversation
Hi @lkarthee, is there anything that can help the JSON makeup ship? |
Ah, good call. We will separate the background from the icon, to make the process easier. Please hold on. :)
Can you please submit a separate PR for this so we get it up ASAP? I assume other languages may require it. |
The |
This is done!
Ack! I meant the change you did will likely be required for EEx/DIFF too (as EEx or DIFF language are also weird). |
Yes, they'd have to also set the |
I just wanted to merge it sooner but we can also wait until someone does/needs the same change. So no worries. |
352de98
to
c2e04d0
Compare
Rebased, removed the change in |
Beautiful, let's see if we can get makeup_json released and ship it. Thank you! Do you want to PR the language.html.eex change or should I go ahead and change it? |
Sorry @optikfluffel @josevalim - I missed notification. @josevalim I have been using makeup_json for sometime - I did not encounter any issues. It has basic tests like I mentioned in this issue - elixir-makeup/makeup_json#4 . |
@lkarthee awesome! Could you please go ahead and release it then? :) |
Sure will do it this weekend. |
Released it on hex. Sorry for the delay. |
TODO
Done
example.json
is copied from www.json.org/example.html.mix.exs
, sincemakeup_json
doesn't seem to be published on Hex yet.I also changedlanguage.html.eex
to use@language.title
instead of the name, because "JSON Language" didn't look right to me.closes #3, when finished