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

feat: add --lite option for git-node land (makes it usable with repos other than nodejs/node) #546

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DeeDeeG
Copy link

@DeeDeeG DeeDeeG commented May 10, 2021

Makes it possible to use git-node land in repositories other than
nodejs/node. This is helpful for producing Node-style commit
footers, and enforcing some Node PR/commit conventions.


Context: Looking into whether git-node land can be used for landing stuff at the node-gyp repo. With this change, the answer would be "yes."

See: nodejs/node-gyp#2286

cc @rvagg thoughts? (I like that git-node land takes some of the work out of using Node-style commit message prefixes, and maintaining commit message footers such as Reviewed-By: and PR-URL:, but dropping all that and just using release-please and "conventional commit"-style commit messages would be even easier.)

Note to node-core-utils maintainers: If node-gyp maintainers aren't interested in this --lite flag feature, then I don't think there is a real-world user for it. In that case, with no real-world users, I wouldn't recommend merging this. Basically, I can only see this being useful if node-gyp maintainers want it (or someone pops up in the comments here to say they want it).

Makes it possible to use `git-node land` in repositories other than
`nodejs/node`. This is helpful for producing Node-style commit
footers, and enforcing some Node PR/commit conventions.
@rvagg
Copy link
Member

rvagg commented May 11, 2021

yeah, could be a reasonable way to go, I still think moving away from this is probably the future for node-gyp, I tend to avoid pushing commits to nodejs/node these days because this process is getting more complex and onerous and the tooling probably only makes it more pleasant for frequent committers who have a good workflow and muscle memory for it.

I'll try and find some time to try this on node-gyp and report back.

@DeeDeeG
Copy link
Author

DeeDeeG commented May 11, 2021

Yeah, the more I use this, the more of a barrier I see it would be to contribution. I have shifted my attention mostly to understanding release-please, at least for node-gyp.

For the record:I tried this on my machine, and it worked for me. I didn't end up pushing the "landed" commit anywhere, but here is a screen recording of it:

Demo.of.running.git-node.land.--lite.mov

Note to node-core-utils maintainers: I still think this PR might be of interest if anyone wants to use git-node land elsewhere besides nodejs/node. There are a few other repos using the Node-style commit message prefixes and footers. But until someone actually wants to use git-node land with those repos, it seems reasonable not to merge this. If interest in this is renewed some time in the future, feel free to ping me about it. I will probably be able to respond to reviews/get this ready to merge if/when that happens.

@DeeDeeG DeeDeeG changed the title feat: add --lite option for git-node land feat: add --lite option for git-node land (makes it usable with repos other than nodejs/node) May 11, 2021
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

Successfully merging this pull request may close these issues.

None yet

2 participants