pulkit requested changes to this revision. pulkit added a comment. This revision now requires changes to proceed.
Thanks for breaking this up. I have some put comments which are mostly doubts, can you explain by replying to them why you did so? INLINE COMMENTS > phases.py:369 > > + rejected = list() > + changes = [set(), set(), set()] why are we maintaining this rejected list? > phases.py:370 > + rejected = list() > + changes = [set(), set(), set()] > + if dryrun: can you explain what this changes list means? > phases.py:371 > + changes = [set(), set(), set()] > + if dryrun: > + getphase = repo._phasecache.phase This conditional looks unnecessary unless I am mistaken. Can you explain why we need this? > phases.py:392 > + changes[phase].update(faffected) > + else: > + for r in affected: We can prevent this else by returning the values early. > phases.py:496 > > -def advanceboundary(repo, tr, targetphase, nodes): > +def advanceboundary(repo, tr, targetphase, nodes, dryrun=None): > """Add nodes to a phase changing other nodes phases if necessary. We should add documentation about the new dryrun argument and the new return values. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3671 To: khanchi97, #hg-reviewers, pulkit Cc: pulkit, mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel