Skip to content
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

Beautify the (parts) of the repository #1092

Open
clamydo opened this issue Sep 13, 2017 · 4 comments
Open

Beautify the (parts) of the repository #1092

clamydo opened this issue Sep 13, 2017 · 4 comments

Comments

@clamydo
Copy link
Contributor

clamydo commented Sep 13, 2017

How do you think about beautifying (parts of) the code in this repository?

I was working on a pull request on toc2, specifically toc2.js and noted that the formatting was nonuniform. Code readability would gain tremendously if we would just run the code through some beautifers available to make the formatting more streamlined. One could use https://github.com/beautify-web/js-beautify for example for JavaScript.

Would you accept partial pull request tuning the formatting? Also making this part of a contributers policy would be great.

@jcb91
Copy link
Member

jcb91 commented Sep 13, 2017

Haha yes, this has certainly crossed my mind, particularly when working on toc2 😉. I even started running things through jscs, but never got round to making a pr because the changes always end up conflicting with other PRs made in the interim.

Do we have a contributor policy @juhasch? I have a vague idea that maybe the organisation says we follow the ipython one, which if memory serves me correctly is something like "formatting PRs are noise" but I may be confused, or just plain wrong.

In brief, 👍 for me, though obviously we'd need to decide on what style we actually enforce...

@juhasch
Copy link
Member

juhasch commented Sep 13, 2017

I guess we should add a contributors policy, thank you for bringing this up.

I believe most projects have a policy of don't fix what ain't broken, i.e. if you work on something, cleaning up/beautifying is fine. Running scripts automatically reformatting otherwise untouched code is not.

I think it is a wise policy to follow, especially considering that we have no tests for the extensions. So any breakage will only be detected by users later on.

@fkjogu So feel free to beautify the parts of toc2 you are working on, but not for the rest of the Javascript stuff, at least not before we have proper tests.

@jfbercher
Copy link
Member

Yes, of course, you can submit a beautified version as a single PR. This can be useful. A difficulty is indeed as indicated by @jcb91 that "but never got round to making a pr because the changes always end up conflicting with other PRs made in the interim".
For toc2, the extension has growth with many additions and has become a bit complex. Perhaps that a refactoring will be useful. I don't think that beautifying will help a lot, but is certainly welcome.

@clamydo
Copy link
Contributor Author

clamydo commented Sep 14, 2017

Cool, I've created a PR. Can update it, when required. Nicer formatting does not replace proper refactoring, but at least, its not beautiful mess 😄.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants