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

Code cleanup #204

Closed
wants to merge 1 commit into from
Closed

Code cleanup #204

wants to merge 1 commit into from

Conversation

darrencl
Copy link
Contributor

@darrencl darrencl commented Mar 8, 2020

Ensure that the code has consistent style by using code formatter, such as JuliaFormatter.

Closes #203

@darrencl
Copy link
Contributor Author

darrencl commented Mar 8, 2020

@tlienart Here's a draft PR to look at JuliaFormatter. I tested this on Constant.jl. If there's any other file you want me to test this with, kindly let me know! :)

@tlienart
Copy link
Collaborator

tlienart commented Mar 8, 2020

did you try playing with the parameters a bit? I like the idea that the formatter would just get "everything in the same style" (though it doesn't seem to like vertically aligning = signs which I find to sometimes help with readability (e.g. in metadata); this is a matter of personal preference though, same with the way it handles brackets).

I kind of wonder whether this is worth the effort. Generally we do want something close to it but also flexibility for whatever may (subjectively) help readability locally.

What do you think? (and thanks!)

@darrencl
Copy link
Contributor Author

darrencl commented Mar 9, 2020

@tlienart The file in this PR is generated from default setting, but here's the list of options that we can modify.

It seems that there is no option to align = vertically and I agree it's nice for metadata or something like that. With that said, we generally only want this in specific block of code and we can always skip the formatter for that block (See here).

@tlienart
Copy link
Collaborator

tlienart commented Mar 9, 2020

Oh that makes sense, well then I'm happy for you to go ahead when you have the time, and use your judgment in terms of where it makes sense to leave the formatting as is, thanks a lot!

@ablaom
Copy link
Member

ablaom commented Aug 25, 2020

I'm closing this old PR to remove clutter. Feel free to re-open.

@ablaom ablaom closed this Aug 25, 2020
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

Successfully merging this pull request may close these issues.

3 participants