Improve template CI: add matrix testing, use bahmutov/npm-install

[agilgur5]

Description

Commits

fix: use @bahmutov/npm-install in templates' CI

• have been using this internally for about 6 months and I've personally
  contributed bugfixes and optimizations to it, so should be good to
  use by general users too

  • it's a good bit less configuration, which is particularly better for
    beginners, but even for advanced users, it means that improvements
    are shared out to all users
  • I also recommended it in an actions/cache issue about needing too
    much config already too, which I wouldn't have done if I weren't
    confident in its usage now

• this properly caches node_modules by using ~/.npm or Yarn's cache dir
  as recommended, instead of node_modules itself, which is not

  • c.f. https://github.com/actions/cache/blob/main/examples.md#node---npm

• it's also not susceptible to the **/yarn.lock issue that was
  reported because it caches the one lockfile directly instead of
  potentially multiple lockfiles

feat: add test matrix to all templates' CI …

• have been using this internally for about a year now, so should be
  good to use by general users too

  • the original proposal to add Actions to all templates also suggested
    adding matrix testing, but that didn't make it into the original PR

• support Node 10, 12, and 14, as well as ubuntu, macOS, and windows
  latest

  • add a note in the docs mentioning the test matrix
    • also fix one doc's missing size-limit link
      • not sure how that got missed

• add clear names to all jobs and steps as well

  • change some to include the matrix variant that is currently running
  • "Begin CI..." was not really clear, use "Checkout repo" instead

ci: make internal job names more consistent w/ templates' …

• Adding a matrix and bahmutov/npm-install brought the templates
  closer to internal, now do the inverse

• Add "Checkout repo" name for checkout Action

  • previously had no name internally

• Reorder config options for matrix to match templates

• Use "Node" consistently, not "node" or "Node.js"

• Also upgrade to actions/checkout@v2 since the templates have had that
  since their inception

Tags

References

• Actions were originally added in Add GH Action configs for each template #457. Proposal that mentions matrix testing is [proposal] add github actions to templates #438
• Internal matrix testing was added in Run tests against Node/OS version matrix #203
• Internal use of bahmutov/npm-install was added in (ci/optim): add caching for yarn install #625
  • Changing templates' caching to use this was also mentioned in deps: upgrade actions/cache to v2 in all templates #750 (comment)
• Upstream refs to bahmutov/npm-install: fix: resolve tar issues by updating to official @actions/cache  bahmutov/npm-install#37, optim: add secondary cache key if lock hash doesn't exist bahmutov/npm-install#43, Too much configuration required actions/cache#94 (comment)

Fixes

This fixes #799 because bahmutov/npm-install isn't susceptible to that issue. Would still like to hear from upstream why they used **/yarn.lock but actions/cache#400 hasn't gotten a response in over a month 😕

• As a result, this also closes fix GitHub Action templates' cache key by removing glob #800, which was made to fix Github Actions templates cache doesn't hit due to hashing bug #799

[agilgur5]

👍 Each template matches with the others, also same # LoC changed, except for react template where size-limit doc fix was made too.

All tests pass, so inverse seems to be working as well

[jaredpalmer]

While I appreciate the idea of this, I am not a fan of this PR. Spinning up this large of a matrix is unnecessary for a significant percentage of tsdx users. I can understand perhaps testing on 3 version of node, but not on also against windows and macos as a default. The latter should just be a recipe in the docs and not the default for every package.

[agilgur5]

Why is it "unnecessary"? Who is "a significant percentage"?

Matrix testing on different platforms and OSes is almost always good to have to ensure compatibility, and the limitation is usually set-up effort (which this removes) or CI minutes (for OSS it's free, otherwise can always remove this section).

As written above, this was proposed in #438 where you didn't have any opposition. This is not a new idea and not my proposal. As I mentioned there too, anything in the templates is much easier to remove than it is to add. I'd rather add most best practices out-of-the-box and let users delete these if they so choose, than make them opt-in and require effort by users to have best practices. The more effort it requires, the less likely it is to happen, and when it comes to best practices, that means less awareness, less compatibility, less safety, etc

[jaredpalmer]

You released 14 (which had breaking changes) without giving me notice and without a single a prerelease. I don’t have time to review every PR on every project of mine, but I read all release notes religiously. Had this (and size-limit) just been in a prerelease I would have suggested changes prior to releasing to everyone, so the argument that “I didn’t see it” is mute.

Anyways, there is a cost to the matrix as it stands now. It slows down PRs and increases action minutes (which cost money on private repositories). I don’t see a big reason why we are even testing in multiple versions of node
out of the box. Node 12 is sufficent. These are all things that can be customized and are heavily project dependent. All that’s necessary is a bit of documentation.

[agilgur5]

This is another comment that was very hypocritical and disrespectful. I literally didn't respond to this for weeks and this ruined a day or several for me. I'll ask again that you 1) read the full context before responding and 2) please consider how your words and actions have and continue to significantly damage my mental health and what that means for me and the community.

---

which had breaking changes

Neither this PR that has matrix testing nor the size-limit PR #705 are breaking... In fact, #705, this, and some others were originally planned for v0.13.x, which is literally called "Template Improvements".

They are also not listed as breaking in the v0.14.0 release... The few changes that are listed as breaking are called "Slightly Breaking", are only changes in dependencies, and I anticipate the vast majority of users (if not all) will have no breakage during upgrade.

without giving me notice

I wrote in March, literally immediately after getting publishing rights that I'd be using milestones. Until recently, there were only 3 milestones, v0.13.x, v0.14.0, and v0.15.0, each was created either in March or early April, and I don't believe I've edited their descriptions or names since then.
I also told you in June repeatedly that there's a very transparent and consistent process to everything, I use milestones, I heavily make use of labels, use RFCs, rotate pinned issues, sort issues with vote tallies, use prefixes, re-write titles, write very descriptive commits and PRs, and that I don't push to master, ever. (and would like to be even more transparent if GitHub's Projects had more project management and planning features such as Epics, or issues in multiple milestones even)

#438 was proposed in January by Shawn, and you literally responded to it yourself with no objections... #705 was in May, and something similar existed in the codebase prior to May 2019, you literally asked for something similar in February, and then again in August, which I responded to and linked to what it duplicated. You did not respond back.
I didn't say you "didn't see it", you literally did see them and either requested or approved of them...

I also literally wrote in the v0.13.3 release that v0.14.0 would be next and literally linked to the milestone there...

And on top of that, after I've merged dozens of PRs, you're requesting to undo mine and contributors' work that you didn't bother to review or respond to for months...
That's not how planning works and is frankly disrespectful to the time contributors and reviewers have already put in.
The corporate analogy here is "someone from up top jumping in and asking a team to change a variety of things despite not being in project discussions for many months and despite previously approving for and asking for said things".

without a single a prerelease [sic]

Can you explain why this deserves a pre-release? This was planned for many months transparently, is well tested (I've made tsdx test dogfood itself, am planning on having tsdx build dogfood itself, have contributed to various upstream and upstream upstream repos, and have written, requested, and coached in more tests than anyone and would've written more if not for things like #407), was blocking progress on many other things, was causing lots of user issues, and all of the breaking changes are necessary. There's nothing experimental in them.
If I have to make a pre-release solely for you to read, that sounds like a problem. I've already told you repeatedly that I'm not making exceptions for you, especially not when I don't even make exceptions for myself. That's a bad role model and sets bad precedent. I strive to treat everyone equally and be transparent to everyone, planning in the open.
And as I've written twice already, they're also only "Slightly Breaking" and won't even impact the vast majority of users. And the changes you have a problem with are, again, not breaking and were originally slated for v0.13.x.

This is also extremely hypocritical. You've made 0 prereleases for TSDX, even though some of these releases were extremely breaking, and some patches were breaking and I've had to point out and fix broken releases before. Users expressed displeasure with those as well. This comes off as "do as I say, but not as I do".

I've also put in lots of stability work as I've written and intentionally made various efforts to make things backward-compatible where feasible, even adding the only deprecation notices that exist in TSDX. You're hypocritically barking up the wrong tree here.
I'll make prereleases for things that are experimental and need extra testing, but I'm not making prereleases just so you can read and give feedback to PRs that were made months prior and have gone through multiple iterations before being merged.

the argument that “I didn’t see it” is mute.

I didn't even say that, but again, you did in fact see them.

But frankly, that's an absurd statement. It's one thing to give feedback, but it's a totally other thing to blame me for the fact that you abdicated responsibility, you don't read any issues, PRs, milestones, releases, or emails, you don't respond to anything, and you didn't ask for anything. That's abusive and a hypocritical double-standard.

Instead of thanking me for being the only maintainer for close to a year -- dedicating ~1000 hours to this and even ignoring some of my other repos, responding to all issues, debugging people's code, reviewing all PRs through multiple iterations and contributing to many of them, organizing, labeling, and deduplicating hundreds, adding all contributors, including issues, PRs, and contributors from well before I was a maintainer, adding lots of labels, adding milestones, being transparent, adding lots of tests, contributing upstream and upstream of upstream, striving to make things more backward-compatible, fixing lots of bugs, adding major features, writing 100+ PRs -- all without anyone asking me or telling me anything (zero direction) -- the few times you jump in you're seemingly constantly telling me that I'm the problem and that I'm not doing enough, even when you didn't do those things yourself...

[agilgur5]

Anyways, there is a cost to the matrix as it stands now. It slows down PRs and increases action minutes (which cost money on private repositories).

Here is my previous comment:

or CI minutes (for OSS it's free, otherwise can always remove this section).

The increased time of CI on the matrix is negligible for most projects. Really large, performance intensive projects generally require a lot of tuning, especially in CI, so I think that's par for the course there. As I wrote, I'm talking default, best-practices for open-source repos.

I don’t see a big reason why we are even testing in multiple versions of node out of the box. Node 12 is sufficent.

Here is my previous comments:

Matrix testing on different platforms and OSes is almost always good to have to ensure compatibility, and the limitation is usually set-up effort (which this removes)
As I mentioned [in #438,] I'd rather add most best practices out-of-the-box and let users delete these if they so choose, than make them opt-in and require effort by users to have best practices [...]

I think if your primary target is browsers, perhaps you don't need to test on all versions of Node, you'd want to test on multiple browsers instead. Though ensuring install works too is good. And React projects can be agnostic to platform, like Ink is React for CLIs.
I'm open to changing that for the React templates, but the basic one may often be used for Node libraries.

In either case, as I've written elsewhere, the templates should really be treated as starting points and not ending points.

Node 12 is sufficent. [sic]

Why is it sufficient? Node 10 is not EOL and I think if you're only going to test any version, it should be the minimum compatible one (also mentioned in #733, which similarly refers to #438). It's generally good practice to maintain support for EOL versions for some time too to allow for more gradual migrations if it is not overly difficult to do so.

You haven't answered my questions and are stating opinions matter-of-factly without pointing to supporting evidence or backing rationale. That doesn't make for a productive discussion and makes me feel like my opinion doesn't matter.

These are all things that can be customized and are heavily project dependent. All that’s necessary is a bit of documentation.

As I've said, I'm focusing on best practices for open-source repos, that wasn't my proposal (but I agree), and it's significantly easier to remove config then add it. People have in fact been appreciative of the CI config as it saves them time. As I wrote previously:

The more effort it requires, the less likely it is to happen, and when it comes to best practices, that means less awareness, less compatibility, less safety, etc

TSDX itself is all about less config and best practices out-of-the-box, and matrix testing aligns with that.