Submitter | phabricator |
---|---|
Date | Jan. 18, 2018, 1:38 p.m. |
Message ID | <differential-rev-PHID-DREV-hqeiaoxo7b4xwrgeaqcf-req@phab.mercurial-scm.org> |
Download | mbox | patch |
Permalink | /patch/26871/ |
State | Superseded |
Headers | show |
Comments
pulkit added inline comments. INLINE COMMENTS > strip.py:218 > # between the working context and uctx > - descendantrevs = repo.revs("%s::." % uctx.rev()) > + descendantrevs = repo.revs(b"%d::." % uctx.rev()) > changedfiles = [] b'' is not required. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1884 To: durin42, #hg-reviewers Cc: pulkit, mercurial-devel
durin42 marked an inline comment as done. durin42 added inline comments. INLINE COMMENTS > pulkit wrote in strip.py:218 > b'' is not required. It's not, but we're going to eventually want to kill the module loader hack, and so I figured since I'm dirtying the line anyway I may as well b-prefix it. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1884 To: durin42, #hg-reviewers Cc: pulkit, mercurial-devel
pulkit accepted this revision. pulkit added inline comments. INLINE COMMENTS > durin42 wrote in strip.py:218 > It's not, but we're going to eventually want to kill the module loader hack, and so I figured since I'm dirtying the line anyway I may as well b-prefix it. That's a good idea. I will follow the same. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1884 To: durin42, #hg-reviewers, pulkit Cc: pulkit, mercurial-devel
yuja added inline comments. INLINE COMMENTS > strip.py:218 > # between the working context and uctx > - descendantrevs = repo.revs("%s::." % uctx.rev()) > + descendantrevs = repo.revs(b"%d::." % uctx.rev()) > changedfiles = [] It should be `repo.revs("%d::.", uctx.rev())` by the way. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1884 To: durin42, #hg-reviewers, pulkit Cc: yuja, pulkit, mercurial-devel
durin42 marked an inline comment as done. durin42 added inline comments. INLINE COMMENTS > yuja wrote in strip.py:218 > It should be `repo.revs("%d::.", uctx.rev())` by the way. I'm unclear: are you asking to remove the b-prefix? Adding it was intentional, since eventually we're going to need to rewrite everything so we can ditch the module loader hack. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1884 To: durin42, #hg-reviewers, pulkit Cc: yuja, pulkit, mercurial-devel
yuja added inline comments. INLINE COMMENTS > durin42 wrote in strip.py:218 > I'm unclear: are you asking to remove the b-prefix? Adding it was intentional, since eventually we're going to need to rewrite everything so we can ditch the module loader hack. No. I mean it should use revset.formatspec. i.e. `revs(expr, arg)` instead of `revs(expr % arg)`. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1884 To: durin42, #hg-reviewers, pulkit Cc: yuja, pulkit, mercurial-devel
durin42 added inline comments. INLINE COMMENTS > yuja wrote in strip.py:218 > No. I mean it should use revset.formatspec. i.e. `revs(expr, arg)` instead of `revs(expr % arg)`. Oh good catch, I'll insert another change to fix that in isolation. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1884 To: durin42, #hg-reviewers, pulkit Cc: yuja, pulkit, mercurial-devel
Patch
diff --git a/hgext/strip.py b/hgext/strip.py --- a/hgext/strip.py +++ b/hgext/strip.py @@ -215,7 +215,7 @@ # only reset the dirstate for files that would actually change # between the working context and uctx - descendantrevs = repo.revs("%s::." % uctx.rev()) + descendantrevs = repo.revs(b"%d::." % uctx.rev()) changedfiles = [] for rev in descendantrevs: # blindly reset the files, regardless of what actually changed