This is an archive of the discontinued Mercurial Phabricator instance.

strip: use %d for known-int string interpolation
ClosedPublic

Authored by durin42 on Jan 18 2018, 8:38 AM.

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

durin42 created this revision.Jan 18 2018, 8:38 AM
pulkit added a subscriber: pulkit.Jan 18 2018, 8:42 AM
pulkit added inline comments.
hgext/strip.py
218

b'' is not required.

durin42 marked an inline comment as done.Jan 18 2018, 8:45 AM
durin42 added inline comments.
hgext/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.

pulkit accepted this revision.Jan 18 2018, 8:47 AM
pulkit added inline comments.
hgext/strip.py
218

That's a good idea. I will follow the same.

yuja added a subscriber: yuja.Jan 18 2018, 8:53 AM
yuja added inline comments.
hgext/strip.py
218

It should be repo.revs("%d::.", uctx.rev()) by the way.

durin42 marked an inline comment as done.Jan 18 2018, 8:55 AM
durin42 added inline comments.
hgext/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.

yuja added inline comments.Jan 18 2018, 9:07 AM
hgext/strip.py
218

No. I mean it should use revset.formatspec. i.e. revs(expr, arg) instead of revs(expr % arg).

durin42 added inline comments.Jan 18 2018, 10:23 AM
hgext/strip.py
218

Oh good catch, I'll insert another change to fix that in isolation.

indygreg accepted this revision.Feb 1 2018, 3:30 PM
This revision is now accepted and ready to land.Feb 1 2018, 3:30 PM