On 2021-01-21, David Blaikie via llvm-dev wrote:>Definite +1 to this - please include the full history of the patches >commit/reverts, and reasons for them. (eg: "originally committed as <hash> >reverted due to <buildbot info (link and quote ideally, the logs get >cleaned up so the link won't be around forever but the commit >message/quoted errors/etc will be)> recommitted as <hash> with <describe >the fix/pointing to particular source files/changes>, etc... " - I usually >do that in list form: ><hash> originally committed ><hash> reverted due to... ><hash> recommitted with fix... >... >)Some new contributors tend to make reverts without a justification. https://llvm.org/docs/DeveloperPolicy.html#commit-messages actually has a somewhat related sentence but it is probably buried in a long document. "If the commit is a bug fix on top of another recently committed patch, or a revert or reapply of a patch, include the git commit hash of the prior related commit. This could be as simple as “Revert commit NNNN because it caused PR#”." There are some other undocumented good practices, e.g. * If the patch may take some time to reland or miss something more than simple test tweaks, consider reopening the differential and (if requires further review) requesting for changes. * `git cherry-pick`. Make sure `Differential Revision: ` is in the reland commit so that it is connected to the original differential.>On Thu, Jan 21, 2021 at 12:11 AM James Henderson via llvm-dev < >llvm-dev at lists.llvm.org> wrote: > >> One other thing I find useful when relanding a patch after fixing it is to >> include in the new commit message the reason for the breakage/how the new >> patch fixes it. >> >> James >> >> On Wed, 20 Jan 2021 at 21:23, Mehdi AMINI via llvm-dev < >> llvm-dev at lists.llvm.org> wrote: >> >>> >>> >>> On Wed, Jan 20, 2021 at 10:39 AM Paul C. Anagnostopoulos via llvm-dev < >>> llvm-dev at lists.llvm.org> wrote: >>> >>>> This morning I pushed a commit that killed the build, so I reverted it >>>> and pushed the new commit to fix the build. Then I did another revert to >>>> get my changes back so I can work on them some more. >>>> >>>> Is it legitimate to use that second revert commit, which was never >>>> pushed, to do the additional work, changing the title to something >>>> reasonable? If not, could you explain what I ought to do? >>>> >>> >>> That works, in general I tend to `git cherry-pick <original commit>` >>> instead, because it brings back the original commit description with it :) >>> (and possible link to the phabricator revision). >>> I'll still edit the message (git commit --amend) to add that this is a >>> reland after fixes. >>> >>> -- >>> Mehdi >>> >>>> >>>> _______________________________________________ >>> LLVM Developers mailing list >>> llvm-dev at lists.llvm.org >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>> >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>>_______________________________________________ >LLVM Developers mailing list >llvm-dev at lists.llvm.org >https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Paul C. Anagnostopoulos via llvm-dev
2021-Jan-25 22:52 UTC
[llvm-dev] Git question about revert
I haven't found a "reopen differential." What's the right way to do that? At 1/25/2021 05:23 PM, Fangrui Song wrote:>* If the patch may take some time to reland or miss something more than > simple test tweaks, consider reopening the differential and (if > requires further review) requesting for changes.