Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion: CITGM for a module dependants #84

Open
mcollina opened this issue Dec 11, 2018 · 10 comments
Open

Suggestion: CITGM for a module dependants #84

mcollina opened this issue Dec 11, 2018 · 10 comments
Assignees
Labels
pull request wanted Create a PR from the discussion
Milestone

Comments

@mcollina
Copy link
Member

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
Copy link

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

@mcollina
Copy link
Member Author

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

cc @nodejs/citgm

@wesleytodd
Copy link
Member

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 expressjs/express#3813 shows, it wold be helpful to go further into our dependency tree. Further than the core CITGM is meant to do IMO.

@mhdawson
Copy link
Member

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

@dominykas
Copy link
Member

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
Copy link
Member Author

Great work and good idea!

@mhdawson
Copy link
Member

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

@BethGriggs
Copy link
Member

BethGriggs commented Dec 3, 2019

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

@dominykas
Copy link
Member

Thanks! I will definitely take a look!

@dominykas
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pull request wanted Create a PR from the discussion
Projects
Development

No branches or pull requests

8 participants