-
Notifications
You must be signed in to change notification settings - Fork 27
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
Cleanup - Consistent code style #203
Comments
Ha I didn't know of JuliaFormatter, it could be a good idea, do you have experience with it? Also generally in terms of formatting, I think it would be great to generally have a consistent style but I think Anthony would agree with me that we also don't want to be super strict with it as long as the code is readable. But otherwise I'm for it, I'll be happy to review a PR, possibly I would suggest to do this in chunks starting small to begin with to see if there's anything we really don't like and adjusting accordingly 😄 Thanks! |
@tlienart I just tried it last week so haven't had a lot of experience with it. However, so far this seems promising given this is width-sensitive formatter and inspired by other formatters, such as black (python), which I used a lot in the past. For sure, I can submit a PR starting with formatting 1 file and we can look how's the result. 😄 |
As per the contributing guidelines, we should adhere to blue-style.
However, some parts of the code seems to not following the guideline, e.g.
https://github.com/alan-turing-institute/MLJModels.jl/blob/885bcc0a33a86fe2ee46340f20a9d99c351b1251/src/builtins/Transformers.jl#L252-L253
Which according to bluestyle, this should be
If the contributors are expected to follow the guidelines, the code will be inconsistent. For that reason, I think we might need to do clean up at some stage to align with the style.
Maybe the easiest is to run JuliaFormatter? With this, the future contributors can easily run the formatter as well. Although, I think at some parts the formatter still doesn't satisfy BlueStyle? (See this issue)
The text was updated successfully, but these errors were encountered: