Patchwork D8738: mergestate: rename a helpless variable name to bit helpful one

login
register
mail settings
Submitter phabricator
Date July 14, 2020, 12:32 p.m.
Message ID <differential-rev-PHID-DREV-24cs3up7h22526v2ltrp-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/46724/
State Superseded
Headers show

Comments

phabricator - July 14, 2020, 12:32 p.m.
pulkit created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  The old variable name `r` makes it ~impossible to understand what does it mean.
  One can only understand that after going to callers and hoping that its
  documented there.
  
  I also documented return value of the function involved while I was there.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/mergestate.py

CHANGE DETAILS




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

Patch

diff --git a/mercurial/mergestate.py b/mercurial/mergestate.py
--- a/mercurial/mergestate.py
+++ b/mercurial/mergestate.py
@@ -619,7 +619,10 @@ 
         return self._stateextras.setdefault(filename, {})
 
     def _resolve(self, preresolve, dfile, wctx):
-        """rerun merge process for file path `dfile`"""
+        """rerun merge process for file path `dfile`.
+        Returns whether the merge was completed and the return value of merge
+        obtained from filemerge._filemerge().
+        """
         if self[dfile] in (MERGE_RECORD_RESOLVED, MERGE_RECORD_DRIVER_RESOLVED):
             return True, 0
         if self._state[dfile][0] == MERGE_RECORD_MERGED_OTHER:
@@ -660,7 +663,7 @@ 
                 f.close()
             else:
                 wctx[dfile].remove(ignoremissing=True)
-            complete, r, deleted = filemerge.premerge(
+            complete, merge_ret, deleted = filemerge.premerge(
                 self._repo,
                 wctx,
                 self._local,
@@ -671,7 +674,7 @@ 
                 labels=self._labels,
             )
         else:
-            complete, r, deleted = filemerge.filemerge(
+            complete, merge_ret, deleted = filemerge.filemerge(
                 self._repo,
                 wctx,
                 self._local,
@@ -681,12 +684,12 @@ 
                 fca,
                 labels=self._labels,
             )
-        if r is None:
-            # no real conflict
+        if merge_ret is None:
+            # If return value of merge is None, then there are no real conflict
             del self._state[dfile]
             self._stateextras.pop(dfile, None)
             self._dirty = True
-        elif not r:
+        elif not merge_ret:
             self.mark(dfile, MERGE_RECORD_RESOLVED)
 
         if complete:
@@ -708,9 +711,9 @@ 
                     else:
                         action = ACTION_ADD
                 # else: regular merges (no action necessary)
-            self._results[dfile] = r, action
+            self._results[dfile] = merge_ret, action
 
-        return complete, r
+        return complete, merge_ret
 
     def preresolve(self, dfile, wctx):
         """run premerge process for dfile