D6659: graft: split graft code into seperate functions

martinvonz (Martin von Zweigbergk) phabricator at mercurial-scm.org
Thu Aug 1 13:42:24 EDT 2019


martinvonz added a comment.


  I just found a comment I wrote a while ago and forgot to submit. Sorry.

INLINE COMMENTS

> cmdutil.py:3495-3508
> +def continuegraftstate(repo, graftstate, opts):
> +    """updates opts based on the interrupted graftstate once
> +    '--continue' flag is called."""
> +    statedata = readgraftstate(repo, graftstate)
> +    if statedata.get('date'):
> +        opts['date'] = statedata['date']
> +    if statedata.get('user'):

I think we generally avoid having functions mutate inputs and instead make them return the new value. The usual reason for mutating instead is for speed, but that should not be an issue here. The side-effect is especially non-obvious (when looking at call site) since the function has a return value (if it had not had one, the reader would probably assume there must be some side-effect).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6659/new/

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

To: taapas1128, #hg-reviewers
Cc: martinvonz, mercurial-devel


More information about the Mercurial-devel mailing list