This is an archive of the discontinued Mercurial Phabricator instance.

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
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

Kwan added a subscriber: Kwan.Jun 23 2019, 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.Jun 24 2019, 8:44 AM
pulkit added inline comments.
mercurial/shelve.py
652

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.Jun 28 2019, 4:01 AM
navaneeth.suresh updated this revision to Diff 15692.
mercurial/shelve.py
652

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

pulkit accepted this revision.Jun 28 2019, 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.Jun 28 2019, 8:50 AM
martinvonz added inline comments.
mercurial/shelve.py
296–304

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
296–304

Doing that right away!

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

@pulkit Changes have been made.

pulkit accepted this revision.Jun 29 2019, 11:47 AM
This revision was automatically updated to reflect the committed changes.
yuja added a subscriber: yuja.Jun 30 2019, 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.Jul 2 2019, 5:42 PM
mercurial/commands.py
6190

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.Jul 4 2019, 11:07 AM
navaneeth.suresh added inline comments.
mercurial/commands.py
6190

Doing that right away!