Page MenuHomePhabricator

rebase: remove now unnecessary logic to allow empty commit when branch changes
ClosedPublic

Authored by mjacob on Fri, Jul 10, 6:49 AM.

Details

Summary

This was a workaround for a bug in the empty commit check in repo.commit(),
where the parent branch name was incorrectly compared with the wdir branch name
instead of the branch name passed via extra. The bug was fixed in D8724.

The workaround was introduced in b2415e94b2f5.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mjacob created this revision.Fri, Jul 10, 6:49 AM

Could you include in the commit message which commit added the code and which commit made it unnecessary? "the previous patch/commit" is an okay way of saying that , IMO, even though it requires the reviewer to queue the two patches right after each other for the description to make sense.

Could you include in the commit message which commit added the code and which commit made it unnecessary? "the previous patch/commit" is an okay way of saying that , IMO, even though it requires the reviewer to queue the two patches right after each other for the description to make sense.

In this case, I could use the Differential revision (D8724) to refer to the other patch. Would that also make sense? (I wonder why you didn’t mention this possibility — is it because the repository should be self-contained?)

Could you include in the commit message which commit added the code and which commit made it unnecessary? "the previous patch/commit" is an okay way of saying that , IMO, even though it requires the reviewer to queue the two patches right after each other for the description to make sense.

In this case, I could use the Differential revision (D8724) to refer to the other patch. Would that also make sense? (I wonder why you didn’t mention this possibility — is it because the repository should be self-contained?)

Yes, but the D8724 will get recorded, so I think that's actually a good way to refer to it. I think we have done that many times before.