Page MenuHomePhabricator

unshelve: handle stripping changesets on interactive mode
ClosedPublic

Authored by navaneeth.suresh on Jul 24 2019, 8:48 AM.

Details

Summary

On interactive mode, changesets on nodestoremove should be
stripped regardless of the shelve is partial or not. This patch
modifies unshelvecontinue() to do that.

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

pulkit added a subscriber: pulkit.Jul 24 2019, 10:48 AM

Can you add tests for this? There is no test which passes or fails because of this change.

I just found that this change is not required while creating D6694. We will be needing the nodes which we are removing for later in case of a partial unshelve.
The stripbased approach of unshelve failed on the change in D6694 on having this change. Should I abandon this?

I just found that this change is not required while creating D6694. We will be needing the nodes which we are removing for later in case of a partial unshelve.
The stripbased approach of unshelve failed on the change in D6694 on having this change. Should I abandon this?

I think this change is required. A nice way to figure out will be to add the test which you added and not make the change in code and see whether do we remove the extra cset or not.

In D6686#98006, @pulkit wrote:

I just found that this change is not required while creating D6694. We will be needing the nodes which we are removing for later in case of a partial unshelve.
The stripbased approach of unshelve failed on the change in D6694 on having this change. Should I abandon this?

I think this change is required. A nice way to figure out will be to add the test which you added and not make the change in code and see whether do we remove the extra cset or not.

Yes, this change is required. D6694 is modified and rebased on top of this change.

pulkit accepted this revision.Aug 6 2019, 7:09 AM
This revision is now accepted and ready to land.Aug 6 2019, 7:09 AM
pulkit added a comment.Aug 6 2019, 7:53 AM

I am not sure why, but test-shelve.t fails for me with some hash changes. Can you have a look?