darcs

Patch 1586 feature: push --rebase

Title feature: push --rebase
Superseder Nosy List bf, ganesh
Related Issues
Status needs-screening Assigned To
Milestone

Created on 2017-08-15.12:36:00 by bf, last changed 2017-08-16.13:50:13 by bf.

Files
File name Status Uploaded Type Edit Remove
feature_-push-__rebase.dpatch bf, 2017-08-15.14:54:54 text/x-darcs-patch
See mailing list archives for discussion on individual patches.
Messages
msg19589 (view) Author: bf Date: 2017-08-15.12:35:59
I am sending this for discussion and experimentation, so it is not yet
for screened.

The typical use case for me is synchronizing darcs repos on different
computers. For instance, I start developing a new feature (like this
one) on one machine, then record a patch with a WIP tag in its name.
When I continue on the other machine, I pull the patch, work further on
it, amend it (often editing the patch description, sometimes more than
once), then push --rebase,  and so on.
msg19590 (view) Author: bf Date: 2017-08-15.12:41:54
One could argue whether --rebase is a good name for the option. Other
candidates are: --suspend, --suspend-conflicts or --suspend-conflicting.

One could also debate whether changing the semantics of --all for rebase
apply is a good idea (especially when hijacking a patch). I personally
think it's no problem, since I never use --all when rebasing manually,
instead I hit the 'a' key when I know I want to apply everything (so I
still get the questions about which patches to suspend and whether I
want to hijack stuff).
msg19591 (view) Author: bf Date: 2017-08-15.12:44:58
Last remark: inheriting interactivity and verbosity for the selection of
the patches to suspend should also be done for darcs rebase pull (for
concistency). This is not yet contained in the bundle.
msg19593 (view) Author: bf Date: 2017-08-15.14:54:54
New bundle with a second patch that adapts the hijack.sh test to the new
behavior. As it turns out, changing applyPatchesForRebaseCmd /does/
change rebase pull as well as rebase apply, so the test case for rebase
pull had to be adapted, too.
Attachments
msg19596 (view) Author: ganesh Date: 2017-08-16.06:03:48
On the UI:

Logically by analogy with the current set of commands, this should 
be called 'darcs rebase push' rather than 'darcs push --rebase'.

I think we have discussed before moving from 'rebase pull/apply' to 
'pull/apply --rebase' and I'd be fine for that to happen. I'm not so 
keen on a mixture.
msg19597 (view) Author: ganesh Date: 2017-08-16.06:42:23
On the behaviour: I spent a while debating whether rebase pull should suspend the 
local or the remote patches. I can't remember all the details, but I think in the 
end it made more sense to rebase the local patches because the expected workflow 
was that you are pulling from an upstream and don't want to edit its patches. Also 
by definition the patches you are pulling are now in two repositories and having 
incompatible versions of them isn't good.

With rebase push it feels harder: with your approach (rebase the remote patches) it 
wouldn't make sense to rebase push to an upstream. But the approach is consistent 
with reducing the chances of incompatible versions, as again by definition the 
patches you are pushing will end up in two repositories. It's also consistent with 
push being a remote apply operation.

So it seems like the right approach, but maybe some warnings to the user would be 
helpful.
msg19598 (view) Author: bf Date: 2017-08-16.13:50:13
Re UI: My intuition for 'push --rebase' instead of 'darcs rebase push'
was that 'darcs rebase' (until now) edits patches in the local repo,
whereas the new command would do rebase (apply) in the remote. This is
reflected in the implementation which is really just a small variation
in the 'remote apply' step of the normal push command. But I guess my
intuition was influenced too much by the internal view of things. I
agree that 'rebase push' is probably more intuitive and consistent from
the user perspective.

Re semantics: It never occurred to me to suspend the patches that are
being pushed. Perhaps this is because I really want something more akin
to 'git push --force', that is, I want to replace a remote patch with a
locally amended version. In fact, I am considering to add an option that
will also 'rebase obliterate' whatever was suspended in the remote repo
in order to achieve exactly that.

Re adding a warning: that sounds like a good idea.
History
Date User Action Args
2017-08-15 12:36:00bfcreate
2017-08-15 12:41:55bfsetmessages: + msg19590
2017-08-15 12:44:59bfsetmessages: + msg19591
2017-08-15 14:54:54bfsetfiles: + feature_-push-__rebase.dpatch
messages: + msg19593
2017-08-16 06:03:49ganeshsetnosy: + ganesh
messages: + msg19596
2017-08-16 06:42:24ganeshsetmessages: + msg19597
2017-08-16 13:50:13bfsetmessages: + msg19598