search

jw3126 / UnitfulRecipes.jl

Plots.jl recipes for Unitful.jl arrays
MIT License
37 stars 10 forks source link

use JuliaFormatter #48

Closed jw3126 closed 2 years ago

jw3126 commented 3 years ago

@briochemc @gustaphe Here is the promised format PR. Personally, I am not married to a particular style and prefer to leaf everything default over tweaking it. Should I tweak the format, add a formatting script or even enforce the format by a @test JuliaFormatter.format(package_root)? Any thoughts?

gustaphe commented 3 years ago

Nice.

Having a @test call change code makes me a bit uneasy. But @test JuliaFormatter.format(package_root, overwrite=false) will simply fail the test if the code isn't formatted.

jw3126 commented 3 years ago

Dang the format test fails on some Windows builds. I think the most pragmatic thing is then to just drop it.

gustaphe commented 3 years ago

(I'll just continue to give my 2c, hope that's okay)

If the build fails I agree this is not worth it, but for the eventuality that you want to at least do this manually once in a while:

Most of @briochemc 's comments are solved by format(pkgdir(UnitfulRecipes), BlueStyle()) (specifically, BlueStyle agrees with us both that kwargs should not have spaces, and it's a bit more friendly to one-liners).

One could also consider formatting only the src directory, as the docs code is more "for show", and likely to be manually optimized for visual effect already.

briochemc commented 3 years ago

These are very good points @gustaphe. @jw3126 would you mind trying this with the BlueStyle() flag?

Also additional minor point: if we are not testing code formatting in the tests, we should drop the extra dependency, right?