Page MenuHomePhabricator

phabricator: fix processing of tags/desc in getoldnodedrevmap()

Authored by dlax on Nov 23 2019, 5:47 AM.



It seems that the previous logic was wrong (it essentially comes
from changeset 3ab0d5767b54 where the result got accumulated instead of
early returned).
First of all, the "continue" in first "if m:" is useless because we're
at the end of the loop. Then, the algorithm seems weird because we will
process all predecessors of a node and possibly override
toconfirm[node] for each of these having a tag (maybe this doesn't
happen, but still). Finally, we would also override toconfirm[node]
when the "Differential Revision: " is found in changeset description.
Maybe this is not a big deal when there is no mix of local tag and
changeset description update?

The logic is changed so that the loop on predecessors stops upon first
match of a tag and so that the changeset description is only checked if
no tag was found. Therefore, toconfirm[node] is only set once.

Diff Detail

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

Event Timeline

dlax created this revision.Nov 23 2019, 5:47 AM
pulkit accepted this revision.Dec 10 2019, 10:43 AM
This revision is now accepted and ready to land.Dec 10 2019, 10:43 AM