bugmonotone - Bugs: bug #17878, Usability: too easy to...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #17878: Usability: too easy to accidentally fork after merge or disapprove

Submitter:  Ludovic Brenta <lbrenta>
Submitted:  Thu 28 Sep 2006 03:26:55 PM UTC
   
 
Category:  command line UI Severity:  3 - Normal
Item Group:  feature request Status:  Fixed
Privacy:  Public Assigned to:  tbrownaw
Open/Closed:  Closed
mtn version --full: 

monotone 0.30 (base revision: 8729c7d9cbb4f638fd97a9951705a196468b11fe)

Jump to the original submission

Sun 16 May 2010 05:58:50 PM UTC, comment #11: 

        - Many commands that change the heads of a branch (approve, disapprove,
          pull, merge, etc) can now take an option "--update". If run from
          a workspace which is based on a head of the branch and has no local
          changes, this option makes these commands update that workspace to
          the new head. If you always want this behavior, you can define the
             get_default_command_options(cmd)
          hook in your monotonerc.

Timothy Brownawell <tbrownaw>
Group Member
Sun 16 May 2010 05:56:50 PM UTC, comment #10: 

What does the fix branch do? Several proposals appeared in this bug report and I'm not sure which one was finally implemented.

Ludovic Brenta <lbrenta>
Group Member
Sun 16 May 2010 05:46:18 PM UTC, comment #9: 

The fix branch for this is merged now.

Timothy Brownawell <tbrownaw>
Group Member
Sun 09 May 2010 04:32:03 PM UTC, comment #8: 

For the record - we already have a global --no-workspace option already - no need for another --update-workspace one... and remember that you can also configure default command options in monotone, so "your" disapprove could internally always be called with --no-workspace and behave just like before.

Having --[no-]something options would still be cool, though.

Thomas Keller <tommyd>
Group administrator
Sun 09 May 2010 02:24:58 PM UTC, comment #7: 

please don't make 'disapprove', or any other db command, affect the current workspace.

I often perform operations on the database from a workspace, just so I don't have to specify --db or --branch. But that doesn't mean I want that workspace changed.

This is one of the best features of monotone; separating repository operations from workspace operations. Let's keep it that way!

Your workflow is not my workflow; mtn should let us each customize things to support our own workflows nicely.

If your workflow does want some db commands to update the workspace, we need an --update-workspace option to enable that.

And, in general, support for option overwriting via --[no-]*, so you can put --update-workspace in the defaults, and override it on occasion.

Or the ability to write Lua wrappers to do the merge and update; that requires 'automate merge' (we have 'automate update').

For the original issue of avoiding unwanted divergence, change your workflow to always do 'mtn status; mtn heads' before 'commit'. Maybe 'status' could be enhanced to show heads info. Better to implement 'automate commit', and write a Lua wrapper to do the status display and user query before commit.

Stephen Leake <stephen_leake>
Group Member
Sun 09 May 2010 09:54:59 AM UTC, comment #6: 

That seems reasonable.

Thinking more about this, "disapprove" should probably update to the parent revision of the disapproved revision; that would match the expected workflow (i.e.

A
|\
B \
|  \
d  fork

If you disapprove B (creating d), this probably means you want to go back to A and create the fork revision).

If the disapproved revision has no or multiple parents, do nothing and emit a warning.

Ludovic Brenta <lbrenta>
Group Member
Sun 09 May 2010 12:31:36 AM UTC, comment #5: 

Oh, hm, it doesn't.

I don't particularly like the idea of making 'commit' become unpredictably interactive. It means there's an extra step between "commit, go back to coding" which is "wait to see if 'commit' asked anything", and it breaks "run 'make check && mtn ci -m foo && mtn sy', go do something else for a while while that runs", and it just doesn't seem that good an idea to have it be "you have to watch this to see if it needs input, but it usually doesn't".

So that leaves the other option, having 'merge', 'disapprove', 'merge_into_dir', 'propagate', and maybe 'explicit_merge', 'pull', 'sync', 'approve', and 'suspend' update the workspace (and take a --no-update flag to turn this off). But I like having things be orthogonal too (and suspect it helps with learning the concepts), and this would break that somewhat.

I suppose practicality is probably more important here than prettiness, so what would be good criteria for updating? Say, the base revision goes from being a head to not being a head (if it's already not a head, presumably the user did 'update -r ...' and wants it to stay there) and doesn't contain any uncommitted changes (making them possibly go through conflict resolution when they didn't ask for it seems impolite)? This still leaves the potential for confusion from having multiple workspaces (only one would be updated) and different behavior of non-workspace commands based on being in a workspace and on accidentally not being at a head revision (say because your last pull was done outside a workspace). Does the extra convenience outweigh the potential for extra (different) confusion?

I suppose I'll add '--update' and '--no-update' flags for those commands, and we can go bikeshed later on which should be the default.

Timothy Brownawell <tbrownaw>
Group Member
Sat 08 May 2010 11:40:04 PM UTC, comment #4: 

In the scenario I described, "disapprove" cannot warn when it creates divergence because it does not create divergence.  Indeed, it is "commit" that later creates the divergence.

IIUC, adding a message in the changelog works only if the user does not pass -m "comment" to "commit" (which, personally, I do quite often); -m would defeat your helpful message.

Therefore, -1 on your proposed solution :)

What is wrong with my two proposed solutions?

Ludovic Brenta <lbrenta>
Group Member
Sat 08 May 2010 10:29:45 PM UTC, comment #3: 

+1 for that, but I'd personally wait with the second point until Derek landed the changelog editor branch in mainline (or if you want to go for it, base your work on his branch).

Thomas Keller <tommyd>
Group administrator
Sat 08 May 2010 10:19:52 PM UTC, comment #2: 

I think what we want to do here is:

  • 'disapprove' warns if it creates divergence
  • 'status' (and the pre-filled comments in the commit message editor) notes whether the current revision is a head of the current branch or a commit would create divergence
Timothy Brownawell <tbrownaw>
Group Member
  Spam posted by anonymous
Thu 28 Sep 2006 03:26:55 PM UTC, original submission:  

It is too easy to overlook the warning Monotone gives after a merge:

mtn: note: your workspaces have not been updated

If now you change something in your workspace and commit, you will of course get:

mtn: note: this revision creates divergence

That is probably not what you wanted if you've just performed a merge.  While Monotone is self-consistent, I think it violates the Law of Least Astonishment.

The same thing happens if you do:

$ mtn disapprove h:
(change something)
$ mtn commit
...
mtn: note: this revision creates divergence

Again, probably not what you wanted or expected.  Even worse, "disapprove" did not give the same warning that "merge" did.

I can think of two possible solutions:

1. Several commands like "merge" and "disapprove", if run from within a workspace, would automatically update the workspace, or at least offer to do so.

2. "commit" should warn about the divergence before committing, not after; and it should ask for confirmation, like this:

$ mtn commit -m "some message"
...
mtn: note: this revision will create divergence; do you agree? (y/n)

There could be a --force flag to commit, which would disable the warning and confirmation prompt.

Personally, I prefer the second solution, as it keeps "update" orthogonal to other commands, while still giving the user a chance not to create unwanted divergence.

--
Ludovic Brenta.

Ludovic Brenta <lbrenta>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by stephen_leake (Posted a comment)
  • -email is unavailable- added by lbrenta (Posted a comment)
  • -email is unavailable- added by tommyd (Posted a comment)
  • -email is unavailable- added by tbrownaw (Posted a comment)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2010-05-16 tbrownaw StatusNone Fixed
        Open/ClosedOpen Closed
    2010-05-08 tbrownaw Assigned toNone tbrownaw

    Back to the top

    Powered by Savane 3.13-599c.
    Corresponding source code