Update node-core-utils and use autorebase - #34969
Conversation
Sorry, something went wrong.
|
Is "proper" in This will allow to land commits with multiple commits and also properly handle proper `fixup` commits a typo? I'm not a native speaker but the sentence sounds better without it to me. |
Sorry, something went wrong.
|
"properly handle proper" does sound weird, but correct. Without "proper" the meaning would change. I don't think it matters that much for the commit message though. @lundibundi I just realized we should also update the documentation. We should remove the "one commit" limitation on https://github.com/nodejs/node/blob/master/doc/guides/commit-queue.md#current-limitations and add a note somewhere explaining how the rebase works (that the PR will only land if all commits are either following the commit message guidelines or are fixups following the format expected from --autosquash) |
Sorry, something went wrong.
|
@lpinca now that I read it it here it does sound weird but is "correct" 😄 , basically, the idea was - I'm open to suggestions but I'd try to keep the proper `fixup` somehow to avoid the misunderstanding that anything with fixup! prefix in commit message works. (though we could make it work with either nodejs/node-core-utils#490 or manually squashing fixups into first non-fixup commit before them) @mmarchini oh, good catch. I'll try to update the branch soon. |
Sorry, something went wrong.
|
Ok I understand.
This is not clear though. Not sure if it makes sense to specify it in commit message. |
Sorry, something went wrong.
|
Commit message still looks good to me, but if the wording is a problem or hard to understand we can change it to: "properly |
Sorry, something went wrong.
|
Too bad we can't use this PR to land this PR 😅 |
Sorry, something went wrong.
PR-URL: nodejs#34969 Reviewed-By: Mary Marchini <oss@mmarchini.me> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Shelley Vohr <codebytere@gmail.com>
This will allow to land commits with multiple commits and also properly handle proper `fixup` commits. Refs: nodejs/node-core-utils#473 PR-URL: nodejs#34969 Reviewed-By: Mary Marchini <oss@mmarchini.me> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Shelley Vohr <codebytere@gmail.com>
PR-URL: #34969 Reviewed-By: Mary Marchini <oss@mmarchini.me> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Shelley Vohr <codebytere@gmail.com>
This will allow to land commits with multiple commits and also properly handle proper `fixup` commits. Refs: nodejs/node-core-utils#473 PR-URL: #34969 Reviewed-By: Mary Marchini <oss@mmarchini.me> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Shelley Vohr <codebytere@gmail.com>
PR-URL: #34969 Reviewed-By: Mary Marchini <oss@mmarchini.me> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Shelley Vohr <codebytere@gmail.com>
This will allow to land commits with multiple commits and also properly handle proper `fixup` commits. Refs: nodejs/node-core-utils#473 PR-URL: #34969 Reviewed-By: Mary Marchini <oss@mmarchini.me> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Shelley Vohr <codebytere@gmail.com>
PR-URL: #34969 Reviewed-By: Mary Marchini <oss@mmarchini.me> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Shelley Vohr <codebytere@gmail.com>
This will allow to land commits with multiple commits and also properly handle proper `fixup` commits. Refs: nodejs/node-core-utils#473 PR-URL: #34969 Reviewed-By: Mary Marchini <oss@mmarchini.me> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Checklist
/cc @nodejs/node-core-utils
I used build subsystem as in original commit-queue PR, though would tools be better?