add minimal config for JuliaFormatter with yas style

[kalmarek]

@nalimilan

vscode is going to pick this file up and apply the formatting

[ararslan]

Hi @kalmarek, thanks for the PR! JuliaStats repositories tend to follow the style used in Base, which has no set style guide but empirically aligns with many of the rules specified by YASGuide. We could try toggling a few options atop the style setting to better match the current style. Off the top of my head, I think this should get us most of the way there:

style = "yas"
short_to_long_function_def = false
always_use_return = false

[kalmarek]

this is the only thing I could find about Base formatting: https://github.com/JuliaLang/julia/blob/master/CONTRIBUTING.md#code-formatting-guidelines

is there something more specific?

just by looking at HypothesisTests.jl we should also include

import_to_using = false
always_for_in = false

but still there are a few places (if ... else ... end in one line, bunch of whitespace) which do not follow it directly

[ararslan]

I'd favor keeping those options set to true but if the goal is to minimize the diff with existing code then you're right, there are unfortunately some uses of import and for without in.

[kalmarek]

@ararslan if we are ok with creating a single commit reformatting all the src and test
then minimizing diff is not so important.

If so I could also add a gh action which checks formatting of every pull

[nalimilan]

I'd say minimizing the diff is important as it keeps the git history simpler (easier to track changes). It would already be a great improvement to automate code formatting (in this package but also in other JuliaStats packages...), even if we tolerate some variation e.g. regarding the use of in.

[kalmarek]

checking formatting automatically is as simple as adding this action

name: format-check
on:
  push:
    branches:
      - master
      - release-*
  pull_request:
    types: [opened, synchronize, reopened]
jobs:
  build:
    runs-on: ubuntu-latest
    steps:
      - uses: julia-actions/setup-julia@latest
        with:
          version: '1'
      - uses: actions/checkout@v1
      - name: Format check
        shell: julia --color=yes {0}
        run: |
          using Pkg
          Pkg.add(PackageSpec(name="JuliaFormatter", version="0.22.10"))
          using JuliaFormatter
          format("src", verbose=true)
          format("test", verbose=true)
          out = String(read(Cmd(`git diff`)))
          if isempty(out)
              exit(0)
          end
          @error "Some files have not been formatted !!!"
          write(stdout, out)
          exit(1)

if .JuliaFormatter.toml is present;
Of course we can't do that until everything is aligned with the style ;)

It seems to me that the style is not really homogeneous, so enforcing any formatting will result in substantial diff (about 1200 - 1500 lines changed)

[nalimilan]

Sounds good. We don't have a bot that would automatically push a commit to fix formatting?

Could you add a commit showing the most minimal diff you can get (choosing the parameters as appropriate), and the a diff when using the standard YAS style? That will allow assessing whether it's worth using custom parameters at all.

[devmotion]

We don't have a bot that would automatically push a commit to fix formatting?

Quite a few packages I know use reviewdog for displaying formatting suggestions in PRs that you can accept in the Github web interface (I added this setup myself in quite a few of those packages). I think something a bit more manual is preferrable over automatic commits as I think the latter will lead to a messier history and conflicts more easily.

[nalimilan]

Interesting. Can you provide links?

[kalmarek]

@nalimilan I don't think that optimizing parameters for minimal diff is worth doing tbh.

I could prepare those example diffs, but they will be large because old parts of the package follow old style newer newer, and the authors were not consistent. Molding the style just to adhere better to the old style seems a bit misguided.
This is one commit and the current (and future) developers will need to live with the style.

[nalimilan]

Yeah it's not worth spending too much time on tweaking parameters. I was just referring to the ones you mentioned above, which disable the stricter rules about for.

[devmotion]

Interesting. Can you provide links?

For instance, https://github.com/devmotion/CalibrationErrors.jl/blob/main/.github/workflows/Format.yml It's e.g. also used by ChainRulesCore and DynamicPPL and other Turing packages.

[ararslan]

I think something a bit more manual is preferrable over automatic commits

Agreed, also because JuliaFormatter can sometimes make incorrect suggestions.

[mschauer]

Please be careful with operator spacing. It's a mess in the package right now but adding spaces everywhere around * and / does not reflect Julia Base conventions.

[devmotion]

adding spaces everywhere around * and / does not reflect Julia Base conventions.

Is there an official guideline where this is mentioned? I am only aware of https://github.com/JuliaLang/julia/blob/master/CONTRIBUTING.md#code-formatting-guidelines which only mentions that whitespace should be used to make the code more readable (no whitespace at the end of a line) but does not enforce or recommend putting no space around e.g. * or /.

[mschauer]

The manual on mathematical operations has it.

By convention, we tend to space operators more tightly if they get applied before other nearby operators. For instance, we would generally write -x + 2 to reflect that first x gets negated, and then 2 is added to that result.

So often Julia code is written x*y + 7 for clarity of precedence and I think should not be automatically transformed into x * y + 7

[nalimilan]

I don't see a setting in JuliaFormatter to change how spacing around operators is handled.

[codecov-commenter]

Codecov Report

Patch and project coverage have no change.

Comparison is base (932eaac) 93.75% compared to head (90edf21) 93.75%.
Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #299   +/-   ##
=======================================
  Coverage   93.75%   93.75%           
=======================================
  Files          28       28           
  Lines        1729     1729           
=======================================
  Hits         1621     1621           
  Misses        108      108           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.