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

Thoughts on next steps #10

Closed joyeecheung closed 6 years ago

joyeecheung commented 6 years ago

At the moment my plan is:

  1. Tests (meanwhile, fixing more TODOs)
  2. CI (Travis?)
  3. Making sure the repo name/npm package name and entries are right
    • I named this node-core-utils because I think a lot of checking and parsing stuff in lib could be useful for other tools, and if we don't want to maintain & document a stable API then putting them into one repo would make things easier.
    • get-metadata as it is now could be split into multiple binaries IMO, and we can make another binary that chains them together.
  4. Create a pipeline that automates things that a collaborator needs to do as much as possible.

Pseudo targets:

  1. land-pr master 12345
    1. Check PR status, warn about stuff
  2. land-pr --continue
    1. Fetch patches
    2. Get the git status ready for a rebase (on a landing-12345 branch)
    3. (Alternatively?) squash commits with [squash] or [fixup] message
  3. (...user does a git rebase to squash/fix nits)
  4. land-pr --continue
    1. Generate metadata
    2. Somehow know what commits should be amended after the rebase (git rev-list upstream/master...HEAD?)
    3. (interactively?) append the metadata to the commits
    4. Call git rev-list upstream/master...HEAD | xargs core-validate-commit
    5. Probably do a bit more summaries, e.g. diffs, logs
  5. land-pr --continue
    1. git checkout master && git reset --hard landing-12345
    2. Stop at this point, let the user do some more checks, ask them to manually trigger git push upstream master
    3. If fails, suggest a git pull --rebase
  6. land-pr --continue
    1. git branch -D landing-12345
    2. Output the landed in <sha>...<sha> comments for copy-and-paste.

(^ can be modified a bit for backporting as well)

To make the pipeline eventually useful for a bot, the core would need to change the workflow a little bit (collaborators need to edit the branch first with a rebase in advance, then kick off the bot).

I am not sure at which stage we can transfer this to the Node.js foundation, I am thinking at least stage 3, maybe 4, because transferring it to the foundation would attract users and we need to get most things right first.

priyank-p commented 6 years ago

@joyeecheung you should put all four steps or all todos of plan in repo's project so we can track them.

joyeecheung commented 6 years ago

@cPhost Good idea, done in https://github.com/joyeecheung/node-core-utils/projects/1 , thanks!

priyank-p commented 6 years ago

@joyeecheung we should use synk.io as we are using npm packages it will warn us about suspicious packages.

joyeecheung commented 6 years ago

@cPhost I think you meant snyk.io ? I don't think security is a top priority for this tool but..yea why not :D

apapirovski commented 6 years ago

@joyeecheung Any thoughts re: using something like https://github.com/nodegit/nodegit so we can do a bit more than we can straight up by executing git commands on the shell?

To expand on that, my ideal world would be one where we can automate almost everything in a very straightforward way for the vast majority of cases. I'm concerned that using just git on the shell leaves us with requiring too many user interactions.

apapirovski commented 6 years ago

Reason I ask is that I'm thinking of working on land-pr over the weekend so I want to make sure my direction is ok with you before I get too deep into it.

joyeecheung commented 6 years ago

@apapirovski Yes nodegit is also what I've been thinking about when I opened this issue. My original thought was this could work with a custom GIT_EDITOR but I am not sure how to implement one, might as well just use the addon.

apapirovski commented 6 years ago

Awesome, I'll plan to work on it this weekend but let me know if you're already doing any work or have anything planned. Don't want to step on each other's toes.

joyeecheung commented 6 years ago

@apapirovski I am baking the step 2 and 4 locally, will probably open an PR over the weekend.

joyeecheung commented 6 years ago

^Although I wouldn't implement them as land-pr at first, they can be separate targets initially.

joyeecheung commented 6 years ago

Just got another idea: we could implement each step as custom commands of git, iterate on that, and put them together as land-pr when they are ready. E.g.

step 2 = git node-apply-pr 12345 + git node-squash-commits step 4 = git node-append-metadata + git node-validate-commits (user can configure the upstream remote in .ncurc)

..they can also choose to drop node- prefixes for less typing.

apapirovski commented 6 years ago

A bit disappointed in nodegit capabilities when it comes to actually manipulating the repository. It seems like we'll need a hybrid approach of executing command line git commands & using nodegit.

joyeecheung commented 6 years ago

@apapirovski If nodegit cannot cover our needs then we might consider not using it at all to avoid extra dependencies..

Personally I like the custom commands approach better because that's what Chromium/v8 uses as well (see git-cl)

apapirovski commented 6 years ago

Yeah, I think that writing slightly more complex bash scripts will make more sense for these than using nodegit. nodegit is also a pain to install since it has to build libgit2 which takes forever. So if we don't need it as a dependency, I'm all for it.

joyeecheung commented 6 years ago

@abouthiroppy Just so you know, the custom commands don't have to be bash scripts, they are just executables. Also we can put the complicated part into Node.js scripts and interact with the bash scripts with stdio.

apapirovski commented 6 years ago

@joyeecheung Oh yeah, I know — sorry, just expressing myself poorly. We'll need a combination of both IMO based on the testing I've done so far locally, and yeah basically comes down to Node interacting with bash scripts.

(Although when one realizes that stuff like rebase is implemented in a shell script it really makes it clear that anything is possible 😆 )

priyank-p commented 6 years ago

@joyeecheung can you let me know about the last todo in readme

Check number of files changed (request pre-backport)

as already opened a PR for v3 support.

joyeecheung commented 6 years ago

git-node-land has landed, closing..