Autoformat bash scripts, yaml files, and markdown files with pre-commit

[manics]

Closes #1899

For discussion:

• shellcheck (first 3 commits): I think it should be useful, and I've configured it to ignore the current warnings from ci/common. It's not automatically installed though by pre-commit (in contrast to the other autoformatters where running pre-commit will auto-download it for you), so I've put it in a separate configuration file and added a second step to test_chart.yaml to run it.

        - uses: pre-commit/action@v2.0.0
          with:
            extra_args: --config .pre-commit-shellcheck.yaml

  It picked up one minor error- ci/common uses bash not sh syntax. Should we keep shellcheck, make it part of the default pre-commit-config.yaml, or drop it?

• helmlint: should be fine

• prettier (last three commits): I've excluded jupyterhub/templates/ (not proper yaml files) and *.md (it makes the diff hard to review so probably best left for a separate commit after this is reviewed). There was minor one error, see 662f5e8 . The other thing to note is that had to override it's default of double quotes to use single quotes instead, this is because chartpress uses single quotes when resetting jupyterhub/values.yaml. Should we go ahead and auto-format markdown files too?

[consideRatio]

Seems good overall to me @manics, thanks for your work!

The other thing to note is that had to override it's default of double quotes to use single quotes instead, this is because chartpress uses single quotes when resetting jupyterhub/values.yaml.

Arrrgh, I'd like to avoid having both double quote and single quote enforced in different contexts. I remember this issue with chartpress, doh! Perhaps we can coerce chartpress with a new behavior instead? I believe I have tried in the past unsuccessfully though.

Should we go ahead and auto-format markdown files too?

Sure, they are prone to formatting errors. We are using MyST markdown, I wonder if there are some linting and or autoformatting or similarly for MyST specifically btw. I'll investigate.

---

Update:

• VSCode extension for MyST!
• MDFormat with pre-commit hooks available. I'm not sure about complexities of using MyST in general for markdown formatting, but I like the idea of using MDFormat as it is seeminlgy being developed to work with MyST in a sensible way, see this issue for example.

Update:

  # Markdown formatting
  - repo: https://github.com/executablebooks/mdformat
    rev: 0.5.5
    hooks:
      - id: mdformat
        additional_dependencies:
          # Autoformat nested Python code: https://github.com/hukkinj1/mdformat-black
          - mdformat-black

          # Autoformat nested json/yaml/toml: https://github.com/hukkinj1/mdformat-config
          - mdformat-config

          # Tolerate GitHub flavored markdown: https://github.com/hukkinj1/mdformat-gfm
          - mdformat-gfm

Update:

mdformat-config did the following, hmm...

1. Single quotes over double.
2. Not indent YAML lists. I want us to stick with indentation. There have been some situations while working with Helm templates where we could either render a list or a dict that made it required to have indentation before - item was printed. Due to that, I'm quite opinionated about indenting lists if we are going to choose one.

    spec:
-     storageClassName: ""
+     storageClassName: ''
      accessModes:
-       - ReadWriteMany
+     - ReadWriteMany

[consideRatio]

@manics chartpress will not disrupt us after jupyterhub/chartpress#115 is merged and released. It will properly preserve no / single / double quotes for Chart.yaml / values.yaml then.

[consideRatio]

Requested action points

• Get fix: preserve quote formatting for image tags also chartpress#115 merged -> cut chartpress release
• Remove the prettier config so we use the default of double quotes
• Update prettier pre-commit hook to use a non-deprecated entry - see comment in https://github.com/prettier/prettier/blob/master/.pre-commit-hooks.yaml

Bonus action points

• Decide on mdformat / prettier for markdown.
  I'm since just now in favor of prettier as we use it for our dedicated yaml files etc.
• Enable use of prettier for markdown also, it takes care of the yaml/json examples in our code blocks and doesn't cause problems even if we use MyST it seems.
• Actuall perform: run pre-commit --all-files

[consideRatio]

@manics is it okay if I --force push to this branch? (changes here: https://github.com/consideRatio/zero-to-jupyterhub-k8s/commits/pre-commit-2)

• I omitted f000c75 specifically and added commits on top of it
• Rebased on master
• Configured pre-commit for markdown, yaml double quotes, bumped non-deprecated version of prettier, bump shellcheck version.

What would remain then:

• Get fix: preserve quote formatting for image tags also chartpress#115 merged -> cut chartpress release
• Update dev-requirements.txt
• Actually run pre-commit and push those changes

[manics]

Go ahead and push

[consideRatio]

@manics this LGTM now, what do you think?

I did a visual inspection of the markdown and is generally happy about the kind of formatting changes there, as well as in the YAML now that we have double quotes which is the default. With prettier, we also automatically get indentation within the yaml syntax highlighted code blocks! We don't get python black formatting with in the python code blocks though.

[manics]

Seems fine to me! Thanks for finishing this off.

[consideRatio]

🎉 thanks for starting it up!

[consideRatio]

@manics it has been a great success to use prettier I think! :D I catch several syntax errors because it fails to autoformat invalid python code! ❤️