Flux.jl
Flux.jl copied to clipboard
Figure out consistent code style
I've seen a few of files now in FluxML repos where the diffs ping-pong back and forth between different styles. Most of the discrepancy seems to be around function and type signature whitespace. Trailing whitespace too, my editor config is set to trim them and a surprising number of core files seem to have them.
ChainRulesCore recently added JuliaFormatter to their CI and it seems to be working well, but even just a couple of sentences in CONTRIBUTING.md would go a long way.
We can add a line to contributing.md, but we follow two spaces and no trailing whitespace. Additionally we use linux file endings. We use whitespace around types and around default arguments too (kwarg = value).
Well that's just the thing, the current codebase is anything but self-consistent! Just going through the top level of src/, https://github.com/FluxML/Flux.jl/blob/master/src/deprecations.jl has no spaces while https://github.com/FluxML/Flux.jl/blob/master/src/functor.jl does. A big advantage of auto-formatting is that it's hard to argue with a machine :smile:, but regardless of whether we add one there should be some sort of rough consensus on this style question.
Yeah deprecations.jl is not following the style and should be fixed.
I'd like to help with this issue. Can you tell me what can I do?
Well, the first step is to get most contributors to agree on a convention :P. You could try doing something like the ChainRulesCore PR linked above and see what people say.
I like four spaces because it matches docstrings/markdown, but I'm happy with Dhairya's suggestion.
Now that there is a consistent code style for Metalhead, could this be extended to the Flux repo as well?
That would be great. Something like https://github.com/JuliaDiff/ChainRulesCore.jl/blob/main/.github/workflows/format.yml would be nice to have too. We can trial this on a smaller FluxML repo if Flux turns out to hit too many formatting edge cases.
I was about to set that up for Metalhead but decided to hold off until https://github.com/domluna/JuliaFormatter.jl/issues/604 is resolved