D3671: advanceboundary: add dryrun parameter

khanchi97 (Sushil khanchi) phabricator at mercurial-scm.org
Thu May 31 01:23:59 EDT 2018


khanchi97 added inline comments.

INLINE COMMENTS

> pulkit wrote in phases.py:369
> why are we maintaining this rejected list?

rejected list will contain those csets which will be rejected during advancing boundary and then return this list so that we can report for rejected csets during dry_run of phase command.

> pulkit wrote in phases.py:370
> can you explain what this changes list means?

yeah, this list is for storing sets of changed csets, ordered from minimum phase to maximum phase. 
For example, changes[0] is a set of those csets whose phase will be changed from --public to --targetphase. Similarly, changes[1] for --draft csets.
Do we need to add some comments to explain this thing?
Or do you want any change here?

> pulkit wrote in phases.py:371
> This conditional looks unnecessary unless I am mistaken. Can you explain why we need this?

I thought we would calculate `rejected` only when we are in dryrun mode.

> pulkit wrote in phases.py:392
> We can prevent this else by returning the values early.

yeah, I will make this change.

> pulkit wrote in phases.py:496
> We should add documentation about the new dryrun argument and the new return values.

okay, I will do this in other patches too.

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