Page MenuHomePhabricator

shelve: move shelve extension to core
ClosedPublic

Authored by navaneeth.suresh on Jun 20 2019, 2:01 PM.

Details

Summary

Until now, shelve was bootstrapped as an extension. This patch adds
shelve on core.

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

Kwan added a subscriber: Kwan.Sun, Jun 23, 7:11 AM

You should probably run hg mv -A hgext/shelve.py mercurial/shelve.py and amend and re-send to preserve history.

@Kwan Thanks for the review. I have updated the patch as you suggested.

pulkit added a subscriber: pulkit.Mon, Jun 24, 8:44 AM
pulkit added inline comments.
mercurial/shelve.py
639

We don't import things from hgext in mercurial. I am not sure why.

I think it will be fine to duplicate clearstatus() from rebase to shelve.py for the movement and add a comment that it's copied.

navaneeth.suresh marked an inline comment as done.Fri, Jun 28, 4:01 AM
navaneeth.suresh updated this revision to Diff 15692.
mercurial/shelve.py
639

Done. Now, shelve will no longer be dependent on rebase.

pulkit accepted this revision.Fri, Jun 28, 8:50 AM

Thanks a lot for working on this. This needs to be rebased on tip of default branch. Also, can you add an entry about shelve extension being moved to core in relnotes/next file.

This revision is now accepted and ready to land.Fri, Jun 28, 8:50 AM
martinvonz added inline comments.
mercurial/shelve.py
297–305

This doesn't seem right. It seems D3693 just forgot to delete clearstatus(). Could you send a patch before this one to do that?

mercurial/shelve.py
297–305

Doing that right away!

navaneeth.suresh edited the summary of this revision. (Show Details)Fri, Jun 28, 2:12 PM
navaneeth.suresh updated this revision to Diff 15700.
navaneeth.suresh marked an inline comment as done.Fri, Jun 28, 2:16 PM
navaneeth.suresh updated this revision to Diff 15701.

@pulkit Changes have been made.

pulkit accepted this revision.Sat, Jun 29, 11:47 AM
This revision was automatically updated to reflect the committed changes.
yuja added a subscriber: yuja.Sun, Jun 30, 12:07 AM

Need to update extensions._builtin so the shelve "extension" is silently
ignored.

Please send a follow up.

In D6553#96119, @yuja wrote:

Need to update extensions._builtin so the shelve "extension" is silently
ignored.
Please send a follow up.

Doing that right away! Thanks @yuja!

pulkit added inline comments.Tue, Jul 2, 5:42 PM
mercurial/commands.py
6207

I just noticed that the function _dounshelve() still has _ prefix. We generally have that to mark if functions are local to that file or class. So can you send a followup and rename that to dounshelve?

navaneeth.suresh marked an inline comment as done.Thu, Jul 4, 11:07 AM
navaneeth.suresh added inline comments.
mercurial/commands.py
6207

Doing that right away!