( )⚙ D594 cmdutil: removing redundant if condition in amend

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D594

cmdutil: removing redundant if condition in amend
ClosedPublic

Authored by singhsrb on Aug 31 2017, 10:01 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHGae92e5c0441c: amend: removing redundant if condition
rHG456380b98775: amend: removing redundant if condition
rHG270a574ccac9: cmdutil: removing redundant if condition in amend
Summary

There is needless checking for the new commit hash not being equal to
the old commit hash. This condition will always be true at this point in the
code path and thus, can be removed safely. This commit removes the redundant
condition.

Test Plan

ran the test suite.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

singhsrb created this revision.Aug 31 2017, 10:01 PM
singhsrb edited the summary of this revision. (Show Details)Sep 1 2017, 6:18 PM
singhsrb edited the test plan for this revision. (Show Details)
singhsrb retitled this revision from cmdutil: remove redundant if block and fix indentation in amend to cmdutil: removing redundant if condition in amend.
singhsrb updated this revision to Diff 1564.
martinvonz added a subscriber: martinvonz.Sep 2 2017, 2:07 AM
martinvonz added inline comments.
mercurial/cmdutil.py
3155–3164

Ah, it's this stuff that makes the check unnecessary, I suppose. So the check was probably made unnecessary in commit a8aba292.

3166–3176

Since you're modifying this area, could you also clean this up using the ui.configoverride() context manager? (Obviously in a follow-up; it's not necessary for this series.)

Closed by commit rHG270a574ccac9: cmdutil: removing redundant if condition in amend (authored by singhsrb). · Explain WhySep 2 2017, 2:22 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/cmdutil.py (15 lines)

Diff 1572

mercurial/cmdutil.py

files=files, files=files,
filectxfn=filectxfn, filectxfn=filectxfn,
user=user, user=user,
date=date, date=date,
extra=extra, extra=extra,
editor=editor) editor=editor)
newdesc = changelog.stripdesc(new.description()) newdesc = changelog.stripdesc(new.description())
if ((not node) if ((not node)
and newdesc == old.description() and newdesc == old.description()
and user == old.user() and user == old.user()
and date == old.date() and date == old.date()
and pureextra == old.extra()): and pureextra == old.extra()):
# nothing changed. continuing here would create a new node # nothing changed. continuing here would create a new node
# anyway because of the amend_source noise. # anyway because of the amend_source noise.
# #
# This not what we expect from amend. # This not what we expect from amend.
return old.node() return old.node()
martinvonzUnsubmitted
Not Done

Ah, it's this stuff that makes the check unnecessary, I suppose. So the check was probably made unnecessary in commit a8aba292.

martinvonz: Ah, it's this stuff that makes the check unnecessary, I suppose. So the check was probably made…
ph = repo.ui.config('phases', 'new-commit', phases.draft) ph = repo.ui.config('phases', 'new-commit', phases.draft)
try: try:
if opts.get('secret'): if opts.get('secret'):
commitphase = 'secret' commitphase = 'secret'
else: else:
commitphase = old.phase() commitphase = old.phase()
repo.ui.setconfig('phases', 'new-commit', commitphase, 'amend') repo.ui.setconfig('phases', 'new-commit', commitphase, 'amend')
newid = repo.commitctx(new) newid = repo.commitctx(new)
finally: finally:
repo.ui.setconfig('phases', 'new-commit', ph, 'amend') repo.ui.setconfig('phases', 'new-commit', ph, 'amend')
if newid != old.node():
martinvonzUnsubmitted
Not Done

Since you're modifying this area, could you also clean this up using the ui.configoverride() context manager? (Obviously in a follow-up; it's not necessary for this series.)

martinvonz: Since you're modifying this area, could you also clean this up using the ui.configoverride()…
# Reroute the working copy parent to the new changeset # Reroute the working copy parent to the new changeset
repo.setparents(newid, nullid) repo.setparents(newid, nullid)
mapping = {old.node(): (newid,)} mapping = {old.node(): (newid,)}
if node: if node:
mapping[node] = () mapping[node] = ()
scmutil.cleanupnodes(repo, mapping, 'amend') scmutil.cleanupnodes(repo, mapping, 'amend')
return newid return newid
def commiteditor(repo, ctx, subs, editform=''): def commiteditor(repo, ctx, subs, editform=''):
if ctx.description(): if ctx.description():
return ctx.description() return ctx.description()
return commitforceeditor(repo, ctx, subs, editform=editform, return commitforceeditor(repo, ctx, subs, editform=editform,
unchangedmessagedetection=True) unchangedmessagedetection=True)