search

nodejs / node-core-utils

CLI tools for Node.js Core collaborators
https://nodejs.github.io/node-core-utils/
MIT License
234 stars 106 forks source link

Moving it into the Node.js foundation? #59

Closed joyeecheung closed 6 years ago

joyeecheung commented 6 years ago

So I have been holding this off because previously we did not have enough test coverage, the README was meh, and we didn't have help text which was confusing. Now that these problems seem to be fixed (thanks for everyone working on this project!) I think it's about time we move this into the foundation.

(I don't think anyone will object, but I may be wrong, so open an issue here first).

targos commented 6 years ago

+1

joyeecheung commented 6 years ago

Also there are a few files missing: LICENSE and CONTRIBUTING.md. I will open a PR later...

priyank-p commented 6 years ago

+1 from me

hiroppy commented 6 years ago

+1

Tiriel commented 6 years ago

LGTM once all the files are here

maclover7 commented 6 years ago

Personally, I'd rather this tool keep iterating separately for a little while outside of the nodejs/automation repo while the dust settles from the monorepo migration. Once everything is in place there, then I'd say we can move it over.

apapirovski commented 6 years ago

I'm +1 once we have all the proper files and "Warn new commits after reviews" is finished.

Also, if we can somehow fix the appveyor auth issues then that would be a part of what I would like to see done (or if not, then have those tests disabled on it). Having green CI makes it a lot easier to attract (and not confuse) contributors.

joyeecheung commented 6 years ago

cc @refack on the Appveyor issue ^...do we just need to disable it on the PR? (Although I can't seem to exclude my own PR before it gets transferred to another owner)

refack commented 6 years ago

Just back in front of a computer. On the AppVeyor thing now. But FYI, there's a limitation on using GitHub apps for repos in the nodejs org (related to access to private repos). Since I assume we want to keep the current integration, need to check the status of that.

Ref: https://github.com/nodejs/nodejs.org/issues/1094

joyeecheung commented 6 years ago

OK I just disabled appveyor on pull requests (disable the pull request event in the webhooks). @refack If you want to work on it, you can push a branch to this repo and open PR with it, or manually trigger the build(not sure how though?)

joyeecheung commented 6 years ago

@maclover7 @apapirovski I understand the concerns, but meanwhile I see no harm in opening an issue in the TSC repo to seek consensus first? The transition can happen later. If you think it's OK I'll open an issue there.

priyank-p commented 6 years ago

@refack @joyeecheung you can jut enable appveyor for fork and then work on it until it gets fixed. Hope that helps.

apapirovski commented 6 years ago

@joyeecheung Sorry, I didn't mean to hold it up 😓 — I was more speaking in terms of what I would like to see happen before an actual transition happens. Of course you should open an issue and get the process started. 👍

(Also in general, assume that any opinions I offer can be taken with a grain salt and are held very loosely. You've done amazing work on this project and I just want to do what I can to help out :) )

Tiriel commented 6 years ago

I was going to ping to see if we should do it once the current PRs are landed, but I see the request has been made already :D

Tiriel commented 6 years ago

I think this can be closed now, since we're in the Foundation!