mapbox / node-pre-gyp

Node.js tool for easy binary deployment of C++ addons
BSD 3-Clause "New" or "Revised" License
1.12k stars 262 forks source link

Needed: New maintainership of node-pre-gyp #657

Closed springmeyer closed 4 months ago

springmeyer commented 2 years ago

I'm stepping down from maintaining node-pre-gyp. For the next month and a half (until the end of June) I can be available to a) facilitate moving maintainer-ship to someone else and b) be available to answer any questions on the transition from that new maintainer or the wider community (on this ticket).

However I will no longer be reviewing PRs, making releases, or responding to bug reports.

As far as a new maintainer, that maintainer could be:

In my mind, all are good options and I will consider any proposals and try to decide promptly (by the end of June) to find a good solution that ensures a) node-pre-gyp is well maintained going forward for the benefit of the entire node.js community and b) also satisfies Mapbox needs.

So, please comment on this ticket if you or your org are interested. Or reach out to me to discuss by emailing me at dane@mapbox.com.

Thanks!


For those curious for extra context and history, here is a quick summary.

Since 2013 I (@springmeyer) have been the primary maintainer of node-pre-gyp. Over this time I've tended to be the single point of contact both for internal Mapbox customers of node-pre-gyp as well as for the wider node.js community (fielding requests from companies using node-pre-gyp for mission critical modules as well as tending to the public github issue queue).

I've recruited help from various Mapbox colleagues over the years (thank you especially @mapsam, and @wilhelmberg) and from countless generous open source contributors (like @Mithgol, @murgatroid99, @brianreavis, @jschlight, and @bmacnaughton). I'm especially grateful to these external folks for helping to keep things chugging during times when I've been in management roles without any time for coding.

Recently I've been away on parental leave and coming back I can see how important it is to find a new maintainer for this module.

The main reasons are:

The quick timeline of node-pre-gyp is that:

/cc @flippmoke @aswamina @danpat @yhahn @mapbox/security-and-compliance

springmeyer commented 2 years ago

In my mind, all are good options and I will consider any proposals and try to decide promptly (by the end of June) to find a good solution that ensures a) node-pre-gyp is well maintained going forward for the benefit of the entire node.js community and b) also satisfies Mapbox needs.

So, please comment on this ticket if you or your org are interested. Or reach out to me to discuss by emailing me at dane@mapbox.com.

Following up here that I did not receive any comments or questions of interest. Also dane@mapbox.com is no longer a valid email as I am no longer an employee at Mapbox and therefore I no longer have maintenance access to this repo. Therefore the time has passed that I would have been able to easily and formally pass on responsibility to others. If anyone finds this issue in the future, and has an interest in taking on maintenance responsibility, I recommend just commenting on this issue (instead of contacting me directly as I previously stated).

ronilan commented 1 year ago

If there is interest from corporate users of the package for more active maintenance and/or tacking some of the more prominent open issues, I may be able to put time/work on this starting spring. Feel free to reach out if relevant.

fcanela commented 1 year ago

Can this unmaintained status be announced in the README.md?

That may help to find new maintainers and quickly understand that the recent issues can not be resolved meanwhile.

cclauss commented 7 months ago

Is there anyone who still has maintenance access to this repo?

I am an active maintainer of node-gyp and would be interested to work with @ronilan (as discussed above) to keep this repository up to date.

At 6.5 million weekly downloads and >1000 dependents (across @mapbox/node-pre-gyp and node-pre-gyp) it's clear this package is well used in the ecosystem...

@petersirota, @yhahn, @willwhite, @mapsam

https://github.com/nodejs/node-gyp/issues?q=is%3Aissue+label%3A%22node-pre-gyp+is+unmaintained%22

Trott commented 7 months ago

As well as being a node-gyp maintainer, @cclauss is a maintainer of Node.js itself. It's probably not necessary, but if it would help, a lot of us on the Node.js team can vouch for his knowledge, diligence, and trustworthiness. I believe there are also people on the Node.js team who are (or at least were at one time) co-workers of @cclauss.

ronilan commented 7 months ago

It's funny waking up to a flurry of activity from a comment made more than a year ago... anyway... I wrote the pull requests mentioned by @springmeyer in the original post (which apparently have rotted a little in the past two years) while on @solarwinds paycheque and using two weeks of their time. I'm (thankfully) not employed there anymore, and while I am way busier than I was when I made the offer @cclauss refers to, if there is a funding, I'd consider.

styfle commented 6 months ago

I am no longer an employee at Mapbox and therefore I no longer have maintenance access to this repo

@springmeyer Seems like this can be solved by forking the repo, however the real issue is npm publish permission.

While you might not have access to publish @mapbox/node-pre-gyp npm package, it looks like you still have access to publish to the original node-pre-gyp npm package.

Perhaps you could grant npm publish permission for node-pre-gyp to @cclauss and we can move forward on a github fork after that? (I'm not sure the origin of the scopeless package if that was actually mapbox or not).

image
mapsam commented 6 months ago

Hi folks, @mapsam here from Mapbox, thanks for engaging here and we really appreciate the eagerness to help maintain node-pre-gyp! The project has sorely missed regular updates from us at Mapbox as the teams' priorities have shifted. I'm putting together a plan to help grant the proper access to those who are interested in maintenance and will respond back when I have more details. We're considering all options right now, from granting NPM access and GitHub access, or even transferring the repository entirely to a new maintainer.

If you'd like to reach out to me directly with any ideas feel free to ping me here in this ticket or email me at sam@mapbox.com in the meantime.

benmccann commented 5 months ago

@mapsam is there any update on transferring maintainership to @cclauss? I see that he has already reviewed and approved several pending PRs, but I'm guessing he doesn't have merge rights since they're still in the queue. Thanks for your help!

mapsam commented 5 months ago

Hey folks, thanks for checking back in. I'd like to get moving on a plan so @cclauss and others can make necessary changes without waiting for a larger plan to go into effect. Very much appreciate the input and response so far! Here's what I'm planning:

Phase 1 - "just get moving again"

Phase 2 - "future home"

Open to feedback here. Cheers!

cclauss commented 5 months ago

Thanks massively @mapsam

I have merged a handful of pull requests that:

  1. remove Travis CI
  2. get Dependabot updating our GitHub Actions and npm dependencies
  3. get AppVeyor testing on modern versions of Node.js
  4. add GitHub Actions tests on Linux, macOS, and Windows

The AppVeyor tests fail so now I think we should only merge PRs that:

  1. Make changes that fix failing AppVeyor and GitHub Actions tests

I am happy to review and merge those pull requests but I would kindly ask that they be as simple to review as possible.

If I create my own pull requests (to add GitHub Actions) who can approve them (I cannot approve my own PRs)?

benmccann commented 5 months ago

If you have a branch with the GitHub Actions setup, I could send it as a PR so you can approve and merge it as a workaround for you not being able to approve and merge your own PRs. (I'm assuming you have only write access to this repo and there are permission restrictions you're facing if I understood your comment correctly)

cclauss commented 5 months ago

Yes. Permissions issue. All pull requests must be approved by a repo maintainer before they can be merged. I can approve and merge your PRs but I cannot approve my own PRs.

benmccann commented 5 months ago

@mapsam a minor suggestion to pass along since I cannot file issues against @mapbox/eslint-config-mapbox. It would be nice if that library switched from the unmaintained eslint-plugin-node to eslint-plugin-n

mapsam commented 5 months ago

Thanks @cclauss! I looked at the branch protection settings for master - here's what I'm seeing. Happy to update so you can move quickly. They don't seem very strict, but given we require reviews I suspect GitHub just makes assumptions that you can't approve your own.

I'll be here for reviews for the moment but will be good to get another maintainer to help you out in the meantime. Happy to include @benmccann if that's helpful, but don't want to put pressure on anyone without their consent and explicit interest! We could always work on feature branches before merging into master as well, that way you can segregate changes of work and not require reviews for every single change.

Screen Shot 2024-07-01 at 10 52 57 AM Screen Shot 2024-07-01 at 10 53 18 AM
mapsam commented 5 months ago

@benmccann I'll get a PR and release out for @mapbox/eslint-config-mapbox in the meantime. I'm not against using an entirely different eslint configuration as well, if that's preferred.

mapsam commented 5 months ago

@benmccann @cclauss @mapbox/eslint-config-mapbox@5.0.1 has been released which uses eslint-plugin-n. Thanks for the tip! https://github.com/mapbox/eslint-config-mapbox/pull/16

benmccann commented 5 months ago

I'm happy to act as a maintainer in the short-term to get a release out. I'm probably not the best maintainer in the long-term as I don't directly use this package. It just happens to be in our dependency tree and so I'm trying to fix some deprecation warnings (mostly by removing packages when possible so that we don't risk the warnings re-occuring), but I'm not very familiar with the package and would be less able to review actual code changes

Thanks for the new release of the eslint config Sam! I sent a PR here to use it

lukekarrys commented 4 months ago

I'd be interested in becoming a long-term maintainer for node-pre-gyp. I'm currently a maintainer of node-gyp and a previous maintainer of npm so I have experience in a similar area to node-pre-gyp.

@benmccann @cclauss @mapsam I am making my way through the recent issues and PRs in this repo to gather context. If there are any specifics that would be helpful for me to focus on initially please tag me on them and I will take a look.

mapsam commented 4 months ago

@lukekarrys lovely, thank you for offering your time and expertise! I've sent you an invitation to be a maintainer of the repo.

cclauss commented 4 months ago

@lukekarrys Welcome to the pre-gyp maintainers team.

First priority is to use the new GitHub Action for releasing to create a pre-release on NPM. That should not be the default release but should allow those who want to ask for it by version number to download it and try it out. After a week or two of listening to that feedback, please repeat the process to make a real production release.

It would be best to do this BEFORE merging other changes because we have done a ton of Dependabot changes without any real world feedback. Small moves….

AFTER the real release is out we need to placate Dependabot and then create another production release. My expertise is Python, not JS so I am ecstatic to have your steadier hands on the wheel. Thanks!

benmccann commented 4 months ago

Thank you @lukekarrys both for volunteering and for all the reviews you've already given.

1.1.0-dev.1 was manually released on July 3. There has been just two small PRs with user visible changes committed since that time. There was one PR to replace rimraf with native node.js APIs. And then one to bump the minor range of the semver dependency. It would be worth giving a second look at this latter PR. It makes the semver range more restrictive without a communicated reason for doing so and has left the project in an inconsistent state because it didn't update the lockfile. We should probably either revert the PR or update the lockfile as first order of business.

I agree setting up the release process is fairly high priority. I think we could make some other changes in parallel since there's already a pre-release version out on npm available for testing containing the majority of prior changes. The other PRs I think are highest priority are replacing npmlog, which would address multiple deprecation warnings being caused by the project's transitive dependencies, and updating dependabot to silence noise in the PR queue. Unfortunately I only have experience with rennovate, so don't have a ton of experience with dependabot. I just returned from vacation last night, so can try to test out the dependabot config changes in my fork this week if you'd prefer to avoid the churn of experimenting with it a bit here. I also have PRs out to upgrade or remove the other direct dependencies that are behind.

cclauss commented 4 months ago

Was there any feedback on 1.1.0-dev.1?

benmccann commented 4 months ago

No complaints so far. I think it'd be nice to merge another batch of PRs and do another pre-release.

cclauss commented 4 months ago

Let’s let @lukekarrys decide.