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

DISCUSSION: Release handling #2860

Closed
rvagg opened this issue May 29, 2023 · 15 comments
Closed

DISCUSSION: Release handling #2860

rvagg opened this issue May 29, 2023 · 15 comments

Comments

@rvagg
Copy link
Member

rvagg commented May 29, 2023

  • I don't have the time, or much reason to care about node-gyp, and would rather not continue to be the gating function to get releases out; I'm only doing this because node-gyp is critical to the ecosystem and nobody else would look after it at a time when I actually cared.
  • There have been a heap of great submissions of late, some of which are significant and too difficult for me to context-switch to review and spend the time to review them. I'm holding up progress on improving this library, it's not fair to contributors or the ecosystem.
  • BUT, the continued security and stability of node-gyp is critical since it has such a wide user base, so there needs to be something in place that ensures a reasonable bar of code quality.
  • One of my challenges has been knowing who to trust, I can only go on code submissions quality, but I don't have the bandwidth for that. I could npm owner add a bunch of names today, but who would that be? I have no idea! Likewise, we could set up auto-releases on merge, which would be great, but then anyone with merge access gets their stuff published and we've had more than a few hiccups on things slipping through in the past few years that needed cleaning up prior to releases.

@cclauss continues to work like a champion answering the firehose of issues that come in because npm install logs always make it look like node-gyp is to blame: https://github.com/nodejs/node-gyp/issues?q=is%3Aissue+is%3Aclosed.

Unfortunately the list of difficult issues keep pilling up: https://github.com/nodejs/node-gyp/issues?q=is%3Aopen+is%3Aissue and the list of great contributions that can't get merged keeps stacking up, with contributors eventually walking away in frustration: https://github.com/nodejs/node-gyp/pulls

DISCUSS

@cclauss @richardlau @targos @StefanStojanovic @lukekarrys @legobeat @DeeDeeG @nodejs/tsc

@legobeat
Copy link

legobeat commented May 29, 2023

Thank you for your efforts, as well as taking the initiative and time to open this discussion @rvagg. We depend on node-gyp in our build process and I would happily consider coming on as a reviewer if this is wanted. IMO it should be preferred with maintainers who have contribution history in the project, where feasible. I sympathize with the difficulty from lack of process (in the very general) for passing the baton for maintainers of ecosystem FLOSS projects.

Likewise, we could set up auto-releases on merge, which would be great, but then anyone with merge access gets their stuff published and we've had more than a few hiccups on things slipping through in the past few years that needed cleaning up prior to releases.

Automated releases sound desirable and I think the issue you describe can be mitigated to some extent via GitHub CODEOWNERS such that a release-PR would have different/stronger requirements for merge
than "normal" PRs changing sources or bumping dependencies.

Slightly OT: Dependabot could also relieve some of the load involved in dependency management, and reduce the risk of falling behind in ways that make later eventual upgrades or releases more difficult.

@legobeat
Copy link

legobeat commented May 29, 2023

You have probably considered this already but one approach would be to add a new group of reviewers and increase the minimum number of required approvals from that team required for merge. This would reduce both blocking-factor of any single individual, while reducing the risk of oversight by individual reviewer resulting in problematic merges/releases.

This could also increase the number of suitable and interested candidates for future maintenance by lowering the barrier to get involved in the contribution process in an impactful way.

@rvagg
Copy link
Member Author

rvagg commented May 29, 2023

You have probably considered this already but one approach would be to add a new group of reviewers

Yep, that would be the best path forward, but who are they and who chooses them? These days I'm too out of touch to know who's who or have much memory for people and their contributions. Even if we went back through recent contributors and put them in the list, do they even care enough? This project has no glory or thanks (except for you doing it just now!), it's at the bottom of the stack and everyone blames it whenever something doesn't compile. It's a hostile workplace and there's very little satisfaction in hanging around this end of things.

I personally like running my projects very open; the whole "OPEN open source" thing I used to promote was the genesis for "open governance" for nodejs/node (make a non-trivial contribution, get commit access). But nodejs/node has two things to make it work well: a gated list of releasers to deal with the problem of bad stuff making from the main branch to people's computers, and a large enough set of eyes to make sure things don't slip through to main in the first place. This project has never had enough eyes, and I have ended up been that gated list of releasers.

@cclauss
Copy link
Contributor

cclauss commented May 29, 2023

As the Python guy, I will say once again that the community would be much better served if the following repos were ported from Python to JS or replaced with something that is JS-based:

@ruyadorno
Copy link
Member

hi @rvagg I'm pretty sure the @nodejs/package-maintenance folks can help you out, iirc they run a program for finding new maintainers to key packages in the ecosystem.

cc @dominykas @wesleytodd @ghinks @ljharb

@wesleytodd
Copy link
Member

We did a few things early on like this, with mixed success. I think one thing that is different here is that a current maintainer is saying "I need help here" vs what we did which was helping to identify critical but struggling packages. I have not been keeping up lately, but would love to hear if we had success with anything other than just putting the word out (which this issue is already doing).

@lukekarrys
Copy link
Member

I would like to become a maintainer of this project if that is possible. The npm CLI team (which I'm currently on) has a large stake in the success of node-gyp, and I think I could help with:

  • Reviewing JS related PRs (Python, not as much). My experience is mostly on the JS side, so it would be ideal to still have maintainers like @cclauss and others who can help with the Python and native side.
  • Dependency updates
  • Discussions around engines, versioning, etc
  • Triaging issues
  • Release management. The CLI and all ~90 of its open source dependencies use the same release management process that could be adopted here. The specific bit about automated releases is something we've tackled by
    1. any merge generates a release PR
    2. GitHub branch protections + a special @npm/cli-team team is used to gate merging release PRs
    3. once a release PR is merged, CI uses a granular access token to publish to the registry

@richardlau I see this was discussed at the TSC meeting today. Is there a process to getting new maintainers nominated/onboarded to node-gyp?

@cclauss
Copy link
Contributor

cclauss commented Jun 14, 2023

Contrary to some of the TSC notes... GitHub says that this repo's code is:
Languages:

  • Python 87.5%
  • JavaScript 10.9%
  • Emacs Lisp 0.9%
  • C# 0.5%
  • C++ 0.1%
  • Shell 0.1%

It would be awesome if we could replace Python code with JavaScript.

@richardlau
Copy link
Member

  • Python 87.5%

@cclauss All of that Python code is a vendored copy from https://github.com/nodejs/gyp-next.

FWIW there was an earlier attempt to replace the Python gyp with Javascript which stalled out

I see this was discussed at the TSC meeting today. Is there a process to getting new maintainers nominated/onboarded to node-gyp?

@lukekarrys The process of finding new maintainers is something the @nodejs/tsc have to figure out. For the record I'd be 100% behind you or any other Node.js package manager maintainers becoming additional maintainers of node-gyp.

@mhdawson
Copy link
Member

@rvagg wouldyou be comfortable with us making @lukekarrys a collaborator in the repo with read/write access. We should also be able to give him npm publishing rights.

@mhdawson
Copy link
Member

mhdawson commented Jun 30, 2023

I've confirmed with @rvagg that provided we are comfortable making @lukekarrys a collaborator then he is ok with that as well.

I propose that we go ahead and make @lukekarrys a collaborator in the node-gyp project with the assumption that he will lead work in the project going forard. Luke works at npm which has a vested interest in the success of node-gyp and we know him well enough to trust him with collaborator rights. @lukekarrys many thanks for volunteering.

@nodejs/tsc any concerns? I'll leave this on the tsc-agenda for another week and if we don't hear any objections by the end of the next TSC meeting I'll go ahead and give @lukekarrys read/write access to the project and work with him to get him what he needs to publish releases to npm.

@mhdawson
Copy link
Member

mhdawson commented Jul 5, 2023

No objections/concerns from the 11 TSC members in the meeting today, so will proceed.

@mhdawson
Copy link
Member

Just added @lukekarrys to the node-gyp team and he should have write access. Will follow up so that we add him in npm as well so that he can do publishes.

@mhdawson
Copy link
Member

Just added @lukekarrys in npm as well. Luke if there is anything else you need let me know.

@lukekarrys
Copy link
Member

I am (finally) ready to to release v9 and v10 here. Apologies for the delays.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants