Wishlist

[jasnell]

fantastic work on this ... loving it.

A couple wish list items that I will try to get to but would love if others get to it first:

• [x] An informational listing of the labels applied to the PR
• [x] Printing the PR title
• [x] Printing the number of individual commits in the PR (useful to know if squashing may be necessary)
• [x] If any commits use [Squash] in their title line, a nice warning saying that Squashing is required
• [x] Print the PR target branch, along with a warning if running get-metadata from within the node source tree and the current git branch does not match the target branch... to help defend against accidentally landing something in the wrong branch.
• [ ] Warning for "Dont Land on ..." labels

[joyeecheung]

On a side note: at this point I think we should rename the binary from get-metadata to something else XD it already does a lot more than generating metadata now..

[priyank-p]

@joyeecheung about renaming binary get-metadata. I have some suggestion. get-pr get-meta

we should probably make it a command binded to package node-core-utils. node-utils get-pr nodejs-automation get-pr | get-meta | get-metadata

[joyeecheung]

@cPhost Actually I kind of prefer get-metadata now that I have used it a few times...because this is supposed to be used when one is landing a PR, usually one would just want to check things while generating the metadata, there aren't really much use cases where one would want to separate those (I could be wrong though). Also technically I think it's OK for something named get-metadata to do a few checks...because those checks are kinda against metadata as well.

Also separating the targets would bring another question: how can we avoid retrieving the data twice? Caches could be invalidated in minutes, and querying the GraphQL API is not that fast at the moment.

[joyeecheung]

@cPhost Giving it a bit of thought I think making a larger target like land-pr in https://github.com/joyeecheung/node-core-utils/issues/10 would make more sense, in that case it's OK to use potentially outdated metadata because

A) This is likely what we are going to do in a complete landing session in practice anyway, last-moment reviews could get omitted if the user does not keep an eye on the browser tab B) This is closer to the semantics of git cherry-pick/git rebase, which is used to land PRs

And get-metadata can stay of course, if the user don't really want to land a PR right now, just want to check the status of it.

[priyank-p]

@joyeecheung That sounds good to me.

[joyeecheung]

@cPhost I do love the idea of piping things tho because I write small bash commands using pipes all the time, it would be great if we can think of some good formats of output that can interact well with other shell utilities..

[joyeecheung]

I've crossed out the first two items because those should be fixed by https://github.com/joyeecheung/node-core-utils/pull/57

[apapirovski]

Third is also taken care of by it :)

[joyeecheung]

I think most items are implemented other than:

Warning for "Dont Land on ..." labels

We do not warn on [Squash] but prompt to run --autosquash if an autosquashable commit (with fixup! or squash!) is present.

[github-actions[bot]]

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.