pre-commit: add initial config and workflow

[Saviq]

• Have you updated CHANGELOG.md with relevant non-documentation file changes?
• Have you updated the documentation for this change?

---

[Saviq]

NB: this is something of a RFC, we can work on improving the config to something that fits better, but the core concept stays.

---

If the app was enabled on this repo, pre-commit would have pushed these automatic fixes itself:

fdd1bfc

[akcano]

yaml?

[akcano]

this is kinda broken now

[Saviq]

Looks like a mdformat problem indeed.

[akcano]

does it work fine with mdformat? not sure myself

[Saviq]

They don't seem to clash, and mdformat seems more opinionated (that's what all the changes here are), but as we see below, it also is not without fault.

I will use whatever you all think is right, just want to spread the use of pre-commit :)

[medubelko]

I'll toss my two cents here, minding the TA and contributor experience. I'm hoping the questions I raise here are helpful to those voices. Maybe others can speak to context behind this work that I'm missing. Please let me know if so. :)

I think we ought to be very deliberate with the purpose and impact of pre-commit in our pack. Have we considered the separation between make lint and pre-commit? Both add friction to the authoring workflow through different strategies. What does it mean if a linter is in one but not the other?

mdformat, like prettier, is highly opinionated. TAs are also opinionated, and it takes us time to come to an agreement on things. For example, I have a principled stance for why hyphens are better list markers than asterisks, but to force all authors to change course on that overnight will take many by surprise, and become an immediate pain point. make lint, by contrast, doesn't get in the way of my work until the CI stage. By introducing pre-commit, we'll be directly insinuating another set of checks, this time forceful ones, into our contributors' daily workflows.

Regardless of what these hooks do, I think we're going to encounter challenges with visibility. Pre-commit has what I consider a major flaw: it doesn't actually communicate well that my commit -- my main unit of work -- has failed. It dumps many lines to the console, potentially a lot of lines if its checks find many problems, then exits. It's then not always clear what I should do next. Do I commit again, in the case of autoformatters (which I have to know ahead of time are in my repo)? Do I go and manually fix the files, hoping that I didn't miss anything? To me this feels like a regression from the concerted effort we put into switching to Vale as a writing aide, where we made its focus be on warning the author, giving them a chance to make a decision, not block them for trivial things. Maybe we could call git status after it runs? But that feels like a band-aid.

As a last but minor point, for repos where the docs are nested in a product, it won't always be obvious how to combine pre-commit configs. We'll need to provide guidance for that.

[Saviq]

I'll toss my two cents here

Thanks for the feedback! I'll try and cluster it, and my answers, in distinct themes, as maybe I should not have mixed things up as much:

mdformat

mdformat, like prettier, is highly opinionated. TAs are also opinionated, and it takes us time to come to an agreement on things.

But whether pre-commit is in use or not, shouldn't affect this?

For example, I have a principled stance for why hyphens are better list markers than asterisks, but to force all authors to change course on that overnight will take many by surprise

The advantage of this is that it can be transparent to them, and if there's agreement on the direction, is there value in not doing it "overnight"?

mdformat is just something that worked for me, so I brought it here under that umbrella. Maybe I should've just brought the whitespace bits to start with ;)

pre-commit

Have we considered the separation between make lint and pre-commit? Both add friction to the authoring workflow through different strategies. What does it mean if a linter is in one but not the other?

As much as possible, I think we should move towards pre-commit. It's a standard approach, less brittle than the Makefile we have. That said, the line may need to be drawn between what can be done on source and on the generated HTML.

make lint, by contrast, doesn't get in the way of my work until the CI stage.

I personally consider that a bug, rather than feature :). You still get the choice, though - you don't have to pre-commit install locally, and leave CI to do its thing.

More, make lint doesn't actually apply the auto fixes (whether local or at CI time), which e.g. might mean I get to come back to failed CI just to clear some whitespace issues…

By introducing pre-commit, we'll be directly insinuating another set of checks, this time forceful ones, into our contributors' daily workflows.

For me it's just one less thing to think about. The tooling takes care of formatting for me.

Regardless of what these hooks do, I think we're going to encounter challenges with visibility. Pre-commit has what I consider a major flaw: it doesn't actually communicate well that my commit -- my main unit of work -- has failed. It dumps many lines to the console, potentially a lot of lines if its checks find many problems, then exits.

Does make lint do better? Maybe I don't get what you mean, but I would rather not commit things that need to be fixed.

It's then not always clear what I should do next. Do I commit again, in the case of autoformatters (which I have to know ahead of time are in my repo)? Do I go and manually fix the files, hoping that I didn't miss anything?

No, it actually applies all the fixes it can - you just need to review and commit them.

As a last but minor point, for repos where the docs are nested in a product, it won't always be obvious how to combine pre-commit configs. We'll need to provide guidance for that.

If a project already has pre-commit, they'll know what to do. If they don't, that's another win in my book - introducing them to it.

vale

To me this feels like a regression from the concerted effort we put into switching to Vale as a writing aide

I don't have the context here, but I also think we should separate tools like vale and just trivial formatting things.

we made its focus be on warning the author, giving them a chance to make a decision, not block them for trivial things.

If it only runs in CI, I won't see the warning anyway, unless it's blocking? And if it is blocking, then how is using pre-commit different?