| Submitter | Angel Ezquerra |
|---|---|
| Date | Nov. 25, 2013, 8:46 p.m. |
| Message ID | <15941436e0ab88dee0f5.1385412408@Angel-PC.localdomain> |
| Download | mbox | patch |
| Permalink | /patch/3135/ |
| State | Superseded |
| Commit | c5aef7a6660769d989344c12299ed773b71dc7bf |
| Headers | show |
Comments
Forwarding Pierre-Yves´s comment to the list: ---------- Forwarded message ---------- From: Pierre-Yves David <pierre-yves.david@ens-lyon.org> Date: Wed, Jan 15, 2014 at 4:03 PM Subject: Re: [PATCH 2 of 4] subrepo: remove unnecessary else clause in hgsubrepo._get To: Angel Ezquerra <angel.ezquerra@gmail.com> On 11/25/2013 12:46 PM, Angel Ezquerra wrote: > > # HG changeset patch > # User Angel Ezquerra <angel.ezquerra@gmail.com> > # Date 1385255580 -3600 > # Sun Nov 24 02:13:00 2013 +0100 > # Node ID 15941436e0ab88dee0f5321f737bcf4bc9774fce > # Parent adb3d2a8dfd8bb1c6531225ef84be55f082bac0f > subrepo: remove unnecessary else clause in hgsubrepo._get > > This revision has no behaviour change. It simply removes an unnecessary else > that follows an if / return block. The change looks big because a big chunk of > code has been unindented one level. I quite love the idea. Not that you could ship a white space ignorant version of your diff too. Not sure what I prefer. -- Pierre-Yves
> From: Pierre-Yves David <pierre-yves.david@ens-lyon.org> > Date: Wed, Jan 15, 2014 at 4:03 PM > Subject: Re: [PATCH 2 of 4] subrepo: remove unnecessary else clause in > hgsubrepo._get > To: Angel Ezquerra <angel.ezquerra@gmail.com> > > > On 11/25/2013 12:46 PM, Angel Ezquerra wrote: >> >> # HG changeset patch >> # User Angel Ezquerra <angel.ezquerra@gmail.com> >> # Date 1385255580 -3600 >> # Sun Nov 24 02:13:00 2013 +0100 >> # Node ID 15941436e0ab88dee0f5321f737bcf4bc9774fce >> # Parent adb3d2a8dfd8bb1c6531225ef84be55f082bac0f >> subrepo: remove unnecessary else clause in hgsubrepo._get >> >> This revision has no behaviour change. It simply removes an unnecessary else >> that follows an if / return block. The change looks big because a big chunk of >> code has been unindented one level. > > > I quite love the idea. > > Not that you could ship a white space ignorant version of your diff > too. Not sure what I prefer. Umm, I'm not quite sure what you mean by that... what is a white space ignorant version of a diff? Cheers, Angel
On 01/16/2014 01:19 PM, Angel Ezquerra wrote: > Umm, I'm not quite sure what you mean by that... what is a white space > ignorant version of a diff? result of `hg diff --ignore-space-change`
On Fri, Jan 17, 2014 at 3:12 PM, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote: > On 01/16/2014 01:19 PM, Angel Ezquerra wrote: >> >> Umm, I'm not quite sure what you mean by that... what is a white space >> ignorant version of a diff? > > > result of `hg diff --ignore-space-change` > Ah, I see. That would make the review better, but the actual patch would still be hard to read so I think it is best to split the patch as I did. Do you agree? Angel
On 01/17/2014 03:35 PM, Angel Ezquerra wrote: > On Fri, Jan 17, 2014 at 3:12 PM, Pierre-Yves David > <pierre-yves.david@ens-lyon.org> wrote: >> On 01/16/2014 01:19 PM, Angel Ezquerra wrote: >>> Umm, I'm not quite sure what you mean by that... what is a white space >>> ignorant version of a diff? >> >> result of `hg diff --ignore-space-change` >> > Ah, I see. That would make the review better, but the actual patch > would still be hard to read so I think it is best to split the patch > as I did. Do you agree? yeah
Patch
# HG changeset patch # User Angel Ezquerra <angel.ezquerra@gmail.com> # Date 1385255580 -3600 # Sun Nov 24 02:13:00 2013 +0100 # Node ID 15941436e0ab88dee0f5321f737bcf4bc9774fce # Parent adb3d2a8dfd8bb1c6531225ef84be55f082bac0f subrepo: remove unnecessary else clause in hgsubrepo._get This revision has no behaviour change. It simply removes an unnecessary else that follows an if / return block. The change looks big because a big chunk of code has been unindented one level. diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py --- a/mercurial/subrepo.py +++ b/mercurial/subrepo.py @@ -651,32 +651,31 @@ urepo = self._repo.unfiltered() if revision in urepo: return + self._repo._subsource = source + srcurl = _abssource(self._repo) + other = hg.peer(self._repo, {}, srcurl) + if len(self._repo) == 0: + self._repo.ui.status(_('cloning subrepo %s from %s\n') + % (subrelpath(self), srcurl)) + parentrepo = self._repo._subparent + shutil.rmtree(self._repo.path) + other, cloned = hg.clone(self._repo._subparent.baseui, {}, + other, self._repo.root, + update=False) + self._repo = cloned.local() + self._initrepo(parentrepo, source, create=True) + self._cachestorehash(srcurl) else: - self._repo._subsource = source - srcurl = _abssource(self._repo) - other = hg.peer(self._repo, {}, srcurl) - if len(self._repo) == 0: - self._repo.ui.status(_('cloning subrepo %s from %s\n') - % (subrelpath(self), srcurl)) - parentrepo = self._repo._subparent - shutil.rmtree(self._repo.path) - other, cloned = hg.clone(self._repo._subparent.baseui, {}, - other, self._repo.root, - update=False) - self._repo = cloned.local() - self._initrepo(parentrepo, source, create=True) + self._repo.ui.status(_('pulling subrepo %s from %s\n') + % (subrelpath(self), srcurl)) + cleansub = self.storeclean(srcurl) + remotebookmarks = other.listkeys('bookmarks') + self._repo.pull(other) + bookmarks.updatefromremote(self._repo.ui, self._repo, + remotebookmarks, srcurl) + if cleansub: + # keep the repo clean after pull self._cachestorehash(srcurl) - else: - self._repo.ui.status(_('pulling subrepo %s from %s\n') - % (subrelpath(self), srcurl)) - cleansub = self.storeclean(srcurl) - remotebookmarks = other.listkeys('bookmarks') - self._repo.pull(other) - bookmarks.updatefromremote(self._repo.ui, self._repo, - remotebookmarks, srcurl) - if cleansub: - # keep the repo clean after pull - self._cachestorehash(srcurl) @annotatesubrepoerror def get(self, state, overwrite=False):