Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Beautify javascript #980

Closed
saivann opened this Issue Jul 26, 2015 · 3 comments

Comments

Projects
None yet
2 participants
Contributor

saivann commented Jul 26, 2015

When merging next changes to javascript code, I planned to use js-beautifier to cleanup the syntax and make bitcoin.org's javascript file a bit more consistent and readable (and maybe it'd be nice to do the same with the CSS files, as I previously used a rather uncommon compact syntax).

Here is a commit that shows the result with current javascript code:
ec4ad7e

It is possible to see only harmless whitespaces are changed with git diff HEAD~1 -w --color-words=.

Contributor

harding commented Jul 29, 2015

I just did a search for "js-beautifier" and found a bunch of different tools with similar names; could you link to the program you plan to use? If it's a Ruby gem, maybe also add it to the development section of the Gemfile.

In general, I think this is a good idea. However, if it's possible, I would prefer setting the indent level to 2 spaces rather than 4.

Contributor

saivann commented Jul 29, 2015

@harding I used the web version of http://jsbeautifier.org/ .

For the record, I didn't intend to include this as an extra step when making changes to the code, as I feel it'd be cumbersome without any real benefit. The goal was more to convert the existing code to a more readable, common and consistent syntax (and I thought this would likely only be reused if we were to stack up enough code changes with inconsistent syntax in the future).

I'm fine with a 2 spaces indent. If you're OK with this, I would "beautify" PR #979 with the parameters you've suggested right before merging it.

Contributor

harding commented Jul 29, 2015

@saivann thanks for the link and for the two spaces indent. Occasional as-needed reformatting is fine with with me.

In general, I think any large change of the site JS should be PR'd rather than direct merged. A formatting-only PR should be trivial to review (you've already provided the command) and I think it could be merged as soon as it was ACK'd by a responsible person.

I'm guessing you want to beautify right before merging to avoid setting off the JS-has-changed notification twice in one day, so how about you push the beautification commit onto #979 now. I'll review it within an hour or so, ACK it, and you can merge it as planned.

@saivann saivann closed this in d814977 Jul 29, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment