nodejs / package-maintenance

Repository for work for discussion of helping with maintenance of key packages in the ecosystem.
Other
407 stars 145 forks source link

Suggestion: CITGM for a module dependants #84

Open mcollina opened 5 years ago

mcollina commented 5 years ago

Developing a library at scale is hard, and as a maintainer the goal is to avoid unwanted breakage and regression. Especially when doing breaking changes, it's of critical importance to validate the impact of those changes, more or less how we do it in core.

In pino, we have done https://github.com/pinojs/pino-integration. In Fastify, we have done https://github.com/fastify/fastify-citgm. I think there is a space to build a generic and supported tool for testing dependant projects, so that we avoid breakages when doing releases.

vweevers commented 5 years ago

We would definitely use such a tool in Level too (we've had https://github.com/Level/abstract-leveldown/issues/184 open for a while, no time to implement it).

mcollina commented 5 years ago

Maybe the engine of https://github.com/nodejs/citgm could be extended to allow testing git checkouts of modules.

cc @nodejs/citgm

wesleytodd commented 5 years ago

This would be great, and I am sure we would want to integrate that into express.

Currently we have some important modules in the core CITGM, but as https://github.com/expressjs/express/issues/3813 shows, it wold be helpful to go further into our dependency tree. Further than the core CITGM is meant to do IMO.

mhdawson commented 5 years ago

I think leveraging CITGM so that maintainers can re-use it is a great idea.

dominykas commented 4 years ago

So here's a proposal on how CITGM for package ecosystems can be implemented.

Basically, the idea is to leverage the existing CI setup, in a similar way that various dependency update management tools do. The implementation could be a bot, which:

  1. Upon a specific trigger phrase in a PR
    • alternative: a commit in a specific branch (e.g. citgm/*, but this may have access control issues1)
    • it should be smart enough to not run citgm on PRs which are citgm tests...
  2. Loops through all the supported dependents / peers
  3. Opens a PR which just updates the package.json to install the package from git
    • this might mean forking some packages, because you don't want to be open PRs just to run your tests in repos that you don't own. The tool could be smart enough to automatically create the forks into a specific org, etc. You might want to do this even for the repos that you own, to reduce noise.
    • the tool should be smart enough to rebase the PRs, etc.
    • sometimes installing from git is not good enough, as there's a build step - the tool could be configured to publish a tarball into GH releases or a separate tool could be used to publish a pre-release on npm
    • it might be possible to get away without opening PRs, but I had a bad experience trying to set that up on Jenkins (might not have been the fault of Jenkins itself...), and the tool should be CI system agnostic. That said - I kind of like the PRs for tracking purposes - they have comments, etc. I know Greenkeeper tries to only open PRs if there's failures, but that might just be rather hard to keep track of and to implement.
  4. The tool then tracks the CI status of all the PRs and reports back to the original PR - via comment and/or via CI status
    • ideally, the tool should be able to compare CI status of the base commit vs the PR commit, to highlight any differences (if there's failures on master, then maybe failing citgm is not a problem, etc; not sure there's common patterns in the ecosystem to get down to individual test level - does Travis even support things like publishing xunit.xml stuff?)
    • once a dependent PR has run its course, the tools should probably just close it

Am I missing something necessary/obvious?

I think the MVP/PoC for the above would be:

  1. A CLI tool, that takes a PR as a param
  2. Loops through supported dependents / peers
  3. Opens PRs, with links to the original PR (so that the original PR gets a notification)

The rest can be build on top of it.


This does differ from the existing approaches in that it does not try to clone/install all of the dependents in a single big task, which does have its pros and cons.

This does not provide a way to run everything locally, but that does not mean it can't be achieved, just that I personally would find less value in my laptop fans spinning, when I can leverage the existing infra.


1 GH branch restrictions only apply to new commits, but not to branch creation, IIRC - which may not be an issue for people with commit access.

mcollina commented 4 years ago

Great work and good idea!

mhdawson commented 4 years ago

@BethGriggs FYI as I know you've been thinking about this as well.

BethGriggs commented 4 years ago

There's a PR over in CITGM at the moment to extend it to test module dependencies -https://github.com/nodejs/citgm/pull/765 I'm still torn between whether this should be an additional feature for CITGM or dedicated tool

dominykas commented 4 years ago

Thanks! I will definitely take a look!

dominykas commented 3 years ago

wiby progress overview: https://github.com/pkgjs/wiby/projects/1