ci: only format and lint changed files

[yanick]

@mcous I have a counter-proposal to #292 It's a small thing, but having any change to the prettier or eslint rules causing a large amount of files to be changed makes me itch. :-) I typically prefer to have the linting done only to the files changed in the PR. The resulting npm scripts are, admitedly, a tad more complex but it'll make for more contained code changes (and a smidge faster checks).

[yanick]

For a project with three actual source files, I think this is needlessly complex and makes the wrong tradeoffs, so I pretty strongly disagree with it, but I'll defer to your judgement

Fair enough. But having a minor change in the prettier configuration results in having 17 files worth of diffs feels worse to me. If it was a one-off instance, I could even push myself to bite the bullet, but this is something that will likely happen each time prettier or eslint fancies change (either changed by us or by these tools' defaults when they'll be upgraded).

[mcous]

My TL;DR on this is that formatting-only changes are easy to review, formatting changes mixed with other changes are hard to review. The more I sit with this, the less I like it. I hope you don't merge it; it'll make my life as a contributor harder and I think it'll make your life as a reviewer, harder, too

Fair enough. But having a minor change in the prettier configuration results in having 17 files worth of diffs feels worse to me

In my experience - including maintaining my company's shared lint configs - I haven't found this to be the case. Not every formatter/lint config affects every file, and I'd much rather skim through a formatting-only PR that is green on CI than have to review a PR that incrementally formats, leaving me with the cognitive load required to pick apart formatting changes from actual behavioral changes.

this is something that will likely happen each time prettier or eslint fancies change (either changed by us or by these tools' defaults when they'll be upgraded).

This is a really good point that I completely missed because I'm not used to working in lockfile-free projects. Setting this PR aside, I think what we need to lock our prettier and eslint versions to avoid uncontrolled drift.

If we decide to intentionally change the settings or update the versions, and it produces code formatting changes, so what? Push up a PR, let CI go green, merge, done. The whole point of having prettier is so people don't have to think about formatting. Once you introduce incremental formatting, you force contributors to think about it

[yanick]

My TL;DR on this is that formatting-only changes are easy to review, formatting changes mixed with other changes are hard to review.

This is true, but the part that I don't like is that files that haven't been actively touched for years will be periodically modified and make the git history more muddy that it needs to be.

At the end of the day, it's all a question of preferences and which trade-offs one is ready to make, and I don't think I'm ready to go with carpet bombing formatting. If somebody else was in charge I would sigh and abide, but right now since I have the keys of the car... ^.^ But since you don't like it, I will refrain from merging this branch for now.

[mcous]

I appreciate that, and I think trying to keep a clean history is a noble goal. Perhaps there is a tooling-lite method (ignore files?) of achieving this goal that we can revisit after some other changes have landed.

In the meantime, I'll pare my prettier PR down to locking the versions and keeping CI formatting checks set to warn rather than error