D3671: advanceboundary: add dryrun parameter
pulkit (Pulkit Goyal)
phabricator at mercurial-scm.org
Wed May 30 17:45:02 EDT 2018
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
More information about the Mercurial-devel
mailing list