D3672: retractboundary: add dryrun parameter

[pulkit (Pulkit Goyal)]

pulkit added inline comments.

INLINE COMMENTS

> phases.py:395
>  
> -    def retractboundary(self, repo, tr, targetphase, nodes):
> +    def retractboundary(self, repo, tr, targetphase, nodes, dryrun=None):
>          oldroots = self.phaseroots[:targetphase + 1]

Add documentation about dry-run and the return value.

> phases.py:417
> +                old = oldroots[targetphase]
> +                affected = set(repo.revs('(%ln::) - (%ln::)', new, old))
> +

Why are we not using this affected set here to find the changesets whose phase is changed?

> phases.py:430
> +            repo.invalidatevolatilesets()
> +        return changes
>  

(Not sure which line I should put this comment on)

Just like https://phab.mercurial-scm.org/D3671, here also, the function should return the correct set of changes whose phase have been changed irrespective of the dryrun value passed.

> phases.py:511
> +
> +    If dryrun is true then it will not perform any action and only returns
> +    set of revision whose phase can be changed.

This should better be:

`If dryrun is True, no actions will be performed

returns a set of revs whose phase is changed or should be changed`

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3672

To: khanchi97, #hg-reviewers
Cc: pulkit, mercurial-devel