Keep intermediate commits

[koeberlue]

It would be nice if you could keep all commits that were not created by mob next.

We are used to make very fine grained commits, like write a failing test, commit, write code to pass test, commit. Discussing commit messages is usually part of the mobprogramming. So it would be really nice to have a way to keep intermediate commits when calling mob done.

Alternatively there could be a way to call mob done without actually ending the mob session. Something like mob commit "some message" which would effectively do:

mob done && git commit -m "some message" && git push && mob start <remaining timer>

This would solve our problem for now but i can see that this probably would not work for all teams.

What do you think? Would this be a useful feature or do the frequent commits just break the flow to much?

[simonharrer]

Hi. Interesting approach. I wouldn’t use frequent commits as it breaks the flow. Not sure how the tool can help here. Also, the tool doesn’t create commits in nonwip branches by design - only in wip branches. I would like to keep it that way. What could help would be a mob next -m „custom message“ parameter. So you can name the commits. And mob done —merge could merge all the commits back to the base branch. What do you think?

[koeberlue]

Hi. Great suggestions. That would really help with our workflow.

Thinkig of it, creating commits only on WIP branches is probably a pretty good idea for most users. I can totally get behind that.

Using mob done --merge in combination with mob next -m "message" would solve almost all problems we are having right how, since we can always do (and keep) a normal git commit inbetween.

I'm just currently thinking about two additions to the approach, which I'm not 100% sure about.

1. When calling mob next without a message, we probably don't want to keep that specific commit. Should the following mob next -m "message" (and mob done --merge) squash all commits containing the default message? If yes, maybe a mob commit -m "message" would be helpful to do the squashing without the need to call mob next -m "message"
2. Should mob done (without --merge) fail, if there are custom commits inbetween? Then one would require an additional parameter to force a squash. So maybe mob done --squash

Both ideas would assume, that all team members have the same default commit message configured, so I'm not quite sure. What's your opinion?

[simonharrer]

Detecting custom commits sounds quite fragile. Don’t like this approach to be honest.

[simonharrer]

Sorry for the short answers but I’m on vacation right now... :-)

[koeberlue]

Detecting custom commits sounds quite fragile. Don’t like this approach to be honest.

Yeah, I totally understand. Wasn't quite sure myself.

So, to sum up, you would be okay with the following additions?

• mob next -m "commit message" would set a custom commit message
• mob done --merge would switch back to the base branch and merge all intermediate commits of the wip branch

Sorry for the short answers but I’m on vacation right now... :-)

No worries. Enjoy your time off and stay healthy out there :)

[simonharrer]

Thanks. :-)

The option —message should also be supported, not only the shorthand.

Regarding the merge option, I am not entirely sure. It still makes a change in the base branch of the user. Maybe this should be done by the user instead? So basically the tool does everything except the actual merge, but displays a command to copy if one wants to do that. Something like: to complete the merge, enter this command and the following commits with those commit messages will be part of your base branch. What do you think?

[simonharrer]

On second thought, the standard mob done also makes a change to the base branch - although only to the index and no commit. So this might be ok, I guess.

[simonharrer]

Back now. Do you want to have a go at the two additions? I'd be happy to review it. :-)

[koeberlue]

Sorry, we had some dead lines looming and I just forgot to answer.

Yeah, I'll give it a try. I don't have that much time currently so this could take a while (also my first time writing in Go).

The option —message should also be supported, not only the shorthand.

Good point. Thinking of it, should there be a shorthand for the --merge as well? -m might be confusing when available for both commands with different meaning.

to complete the merge, enter this command and the following commits with those commit messages will be part of your base branch. What do you think?

Personally I'd prefer the --merge option because it would feel more satisfying (?) when you are done mobprogramming and don't have to fiddle around with some additional git commands.

Thinking of it since mob done removes the wip branch, would a manual merge even be possible?

[simonharrer]

Awesome. Looking forward your PR.

I would refrain for having a shortcut for merge, as this is somewhat dangerous and I don’t want to make this too easy. :-)

[simonharrer]

I want to release a new version soon. So I had a go at the --message option - I hope that's okay with you. Have a look at 4302e89 if you like.

[koeberlue]

Wow, thank you very much! Sorry I didn't find the time for a PR. My son's sleep schedule is currently out of order and I'm really exhausted in my spare time.

Looking at your commit I realise, that I also overestimated the amount of work needed for such a change 😄

Again thank you very much. I hope everything will be more relaxed here in the next few weeks, so I may have a look at the merge option. But of course it's no problem for me if someone else wants to implement it before me :)

[simonharrer]

No worries. I do have two children of my own, and I know how important their sleep schedule is to my well being. :-)

[gregorriegler]

one idea comes to mind:
what if i want to commit 3 times before making a handover.

[koeberlue]

@simonharrer Plesse correct me if I'm wrong, but you should be able to just use a normal git add and git commit for this. mob next should keep those commits

[simonharrer]

mob next won’t interfere - you can commit manually anytime. Just be aware that mob done currently squashes the manual commits as well.

[gregorriegler]

let's say i'm doing 3 microsteps before my handover and i want all those to be kept (not squashed) in my history.
do you see a way to make this possible?

[simonharrer]

that was the idea of having the merge option. Instead of squashing all commits, all will be merged back in the base branch. Something in between (some commits are squashed and some kept) is not really feasible. The merge option is discussed above.

[gregorriegler]

hm. my idea was more like: not detecting custom commits, but detecting mob tool commits.
so the tool is about making it really simple to do git handovers by creating behind the curtain git artifacts that vanish after the fact a.k.a. mob done, no?
it's doing this with the branch, and i'm thinking it could do the same with the commits in the branch.

so you would

1. mob start
2. do a small refactoring
3. git commit -am "r small refactoring"
4. start a new test, but time is over
5. mob next
6. another driver does mob start
7. finishes the test, and adds the easy solution
8. git commit -am "F small feature"
   ... goes on

upon mob done commit from 3. stays. 5. is recognized as a behind the curtain commit and is therefore automagically squashed with 8.

[jbrains]

This seems to work in the meantime:

1. mob start
2. loop a few times:
   1. make a change
   2. commit to mob branch
3. "mob publish": git rebase MOB_BRANCH MAIN_BRANCH && git push && git checkout MOB_BRANCH (assumes both branches are tracked at the remote)
4. loop a few times:
   1. make a change
   2. commit to mob branch

At this point, we have some "published" commits and we have some WIP commits. If you use mob done now, only the unpublished commits are squashed and left to be committed as a batch back to MAIN.

It might help, then, simply to add a mob publish command that rebases/merges then pushes, effectively publishing the inbox of commits as is without squashing.

I don't know the workflow thoroughly enough to feel sure that this always works, but it seems to pass the laugh test for me, so I'll rebase-then-push by hand in my sessions and maybe I'll discover a problem along the way. Can anyone already see a problem? Would this "mob publish" command that I'm proposing help enough to be worth it?

[koeberlue]

@gregorriegler

5. is recognized as a behind the curtain commit and is therefore automagically squashed with 8

I would say the "behind the curtain" and "automagically" part will be a problem in your proposed solution. As far as I understood mob.sh there is no feasible way to detect those intermediate commits created by mob next. At least I can't think of an artifact that we could use to identify those commits at the moment

@jbrains If I understand correctly, git publish would alter the MAIN_BRANCH while the mob session is running. Wouldn't that cause problems when running two mob sessions in parallel?

[koeberlue]

@simonharrer Finally found some time fiddling around. I'm still getting used to Go, so no PR yet ;)

I managed to read in the --merge flag and I felt, that the flag itself was quite unintuitive. One has to understand the inner workings of mob to guess what extactly gets merged into what.

Maybe we should use something like --keep-commits. That would also tell someone reading the documentation, that by default all commits will be squashed. What do you think?

[simonharrer]

I've tried to summarize the goal of this ticket. Basically, there are three modes of using mob:

1. One does not care about commit messages in the commits in the wip branch. One only thinks of commit messages after having issued mob doneand creating a commit for all squashed changes.
2. One does care about commit messages in the wip branch. One doesn't want to lose those commit messages, but doesn't care about the commits themselves. So every commit is either done manually or using mob next --message "asdf". With mob done all changes are merged back, and all commit messages are merged back. Using git commit all previous commit messages are part of the commit message of the single commit made in the main branch by the user.
3. One does care about commit messages in the wip branch. One doesn't want to lose those commit messages and those commits. This is currently not supported through the tool itself.

The tool currently supports 1 and 2. In this part of the ticket, we focus on 3.

One solution is now to add a mob done --keep-commits flag so that instead of squashing all changes and putting them in the index, a git merge is executed from the base branch to the wip branch, keeping all commits.

Things to consider:

• What if there are uncommitted changes in the wip branch when you execute mob done --keep-commits? mob done normally has an implicit mob next included that creates a new commit. This, however, would be going against the clean commit history this user would like to have. So should mob done --keep-commitsonly work when there are no changes? I would intuitively say yes. Other opinions?
• mob done normally doesn't make any commits or moves the branch to a newer commit. With mob done --keep-commits this changes. So basically, these two are very different ways of using mob, with very different behaviors. Just a note to keep that in mind. I just hope that users will not be surprised by that.
• Could the user expect that --keep-commits means something else? Like keep the commits in the mob-session branch instead of destroying the mob-session branch?
• What can happen if the merge fails? Will the mob-session branch be deleted? Or is this now something the user has to take care of? Any consequences when issuing the next mob start? We need tests for those scenarios.
• Should be merge be only fast forward?

@jbrains I'm really hesitant to go in the direction of having such a publish command. A design principle of mob is not to interfere with branches of the user (the base branch), only write/change wip branches. That's basically my biggest concern with the --keep-commits option above - although it only moves the local pointer of a branch forward in the best case. But with your proposal of doing a rebase and a push, this design principle would no longer hold - and I kind of really like this way of branch ownership. It keeps the tool simple and any user with any preferences on how they manage their own branches can use it.

[gregorriegler]

suppose mob done and mob done --keep-commits have a vastly different behaviour, i'd hesitate giving them the same name. but all names are very general though. start, next,... its user-centric

to keep it user-centric i would not throw away a users commit by default. meaning mob done should not do this. in the old case there could be a mob done --stage or --index option or similar.

would probably be a groundbreaking change. but it feels right from a user-perspective at least.

[simonharrer]

@gregorriegler to me, this case of keeping commits is basically a 5% use case whereas having mob manage the short living temporary branch is the 95% case - although I have no data to prove it, just an educated guess. I don't want to optimize too much for the 5% case. I really want to keep the 95% case as simple as possible.

[jbrains]

@simonharrer Since you want mob to stay away from my branches, perhaps the documentation could offer a suggestion on how to avoid the squashing that mob done does. It might be obvious in general, but when I was quickly trying to get mob up and running, I tried mob done, it didn't do what I expected, and I felt momentarily flustered. :)

One other possibility is to have a command like mob done that says "we're done a step (that we want to keep), but we're not ready to hand off to the next typist". I had originally assumed that mob done only referred to the task, and not to the working session, anyway. Maybe distinguishing mob stop (session) from mob done (task) might help?

[koeberlue]

I've tried to summarize the goal of this ticket. Basically, there are three modes of using mob:

1. One does not care about commit messages in the commits in the wip branch. One only thinks of commit messages after having issued mob doneand creating a commit for all squashed changes.

2. One does care about commit messages in the wip branch. One doesn't want to lose those commit messages, but doesn't care about the commits themselves. So every commit is either done manually or using mob next --message "asdf". With mob done all changes are merged back, and all commit messages are merged back. Using git commit all previous commit messages are part of the commit message of the single commit made in the main branch by the user.

Oh, I didn't realize, mob preserves those messages. That's also a very interesting feature 👍

3. One does care about commit messages in the wip branch. One doesn't want to lose those commit messages and those commits. This is currently not supported through the tool itself.

The tool currently supports 1 and 2. In this part of the ticket, we focus on 3.

One solution is now to add a mob done --keep-commits flag so that instead of squashing all changes and putting them in the index, a git merge is executed from the base branch to the wip branch, keeping all commits.

Things to consider:

• What if there are uncommitted changes in the wip branch when you execute mob done --keep-commits? mob done normally has an implicit mob next included that creates a new commit. This, however, would be going against the clean commit history this user would like to have. So should mob done --keep-commitsonly work when there are no changes? I would intuitively say yes. Other opinions?

Intuitively I would expect those uncommited changes to appear as staged changes. We could also print some kind of message telling the user about those uncommited changes. So this would be consistent with the normal done.

• mob done normally doesn't make any commits or moves the branch to a newer commit. With mob done --keep-commits this changes. So basically, these two are very different ways of using mob, with very different behaviors. Just a note to keep that in mind. I just hope that users will not be surprised by that.

Yeah, that behavior should be documented well. But from a user perspective I'd say it is not that different. It is just the way the wip changes are played back into my normal branch. As long as we do not call git push the user should also be confident enough to "repair" any unexpexted behavior.

• Could the user expect that --keep-commits means something else? Like keep the commits in the mob-session branch instead of destroying the mob-session branch?

Good point! I was thinking about something like --no-squash as well, but didn't really like it because it sounds like disabling functionality instead of adding extra steps to the command. For me --keep-commits would describe the expected behavior, but I can also see that it could be misunderstood.

• What can happen if the merge fails? Will the mob-session branch be deleted? Or is this now something the user has to take care of? Any consequences when issuing the next mob start? We need tests for those scenarios.

If the merge fails, I would expect mob to abort the whole operation but also help me resolve the issue. So there should be an error message explaining what went wrong and maybe also a list of commands to execute to resolve the issue. Also it should be consistent with the current bevavior. To be honest I'm not quite sure how conflicts are currently managed. So what happens if we edit a file in a mob-session while someone else removes it from the main branch?

• Should be merge be only fast forward?

I would say no. I'd expect mob to call git merge --no-commit and if there needs to be a merge commit there should be a message telling me about the commit that I have to do. This would be consistent with the case of uncommited changes and also with the usual done command.

[simonharrer]

@jbrains good point. I've updated the documentation and the help info so it becomes clear that it'll do a squash.

Thanks for your input @koeberlue At the moment, mob done issues a git merge --squash. And what we would like is to have a git merge --no-commit instead. So I think mob done --merge squash and mob done --merge no-commit would make this explicit with --merge squash being the default option (although changeable via a configuration option). Maybe that's the way to go? This would also allow this publish operation @jbrains advocates for, although it requires a mob done --merge no-commit && mob start. The possibility to use --merge commit would also work.

[koeberlue]

@simonharrer Great idea! Since "mob is a thin wrapper around git" there is no need to reinvent the wheel. Just give the user a possibility to slightly modify the git commands with flags (potentially) already known to the user. 👍

[simonharrer]

@koeberlue sounds great. I

Anyone interested in doing a mob session to get this feature going?

[koeberlue]

Yes! Should we use the new Github Discussions feature to schedule a meeting? That way we will not spam this issue

[simonharrer]

Let's do this. #92

[gregorriegler]

Maybe something to consider, not sure if I understand this correctly.

When you do git merge --no-commit it merges without creating the "merge commit", and that's all.
So when you would

1. mob start
2. mob next
3. mob done --merge no-commit

you would have the handover commit from (2.) in the mainline. unless you squash it manually.

is this what we are aiming for? or do I not understand?
I thought we're aiming for a design where the handovers and intentional commits are orthogonal

[gregorriegler]

sorry for the accidental close, missclick 🙄

[simonharrer]

Basically, I want to exclude the hybrid approach for now, meaning, one cares about great commits but uses mob next to create temporary commits.

So, I propose to support two modes:

# default mode
mob start
mob next
mob done --merge squash # because it's default, the --merge squash is optional
git commit --message "One message for all changes"

and

mob start
git commit -m "I commit manually"
mob next --message "I care about commit messages" # also possible to do this via mob next
mob done --merge no-commit
git commit --message "One message for the merge commit only" # might not be necessary if fast-forward merge

In the hybrid approach, one would need to do a rebase, probably interactively. This doesn't work well with a CLI tool that's fire and forget as the mob tool currently is.

[simonharrer]

Hm, it could mean that we have two different modes with different arguments being optional or required.

The default mode would have an optional message at mob next whereas the mob next in the second mode requires a message parameter to be present.

[koeberlue]

the mob next in the second mode requires a message parameter to be present

I would argue that this parameter is optional in the second mode as well. If I did meaningful changes, I can add a commit message. If I only want to do the handover on "dirty" changes I will still use the default message. When I dive into the git log I will then see those dirty commits and can just ignore them.

Getting rid of the handover commit, as @gregorriegler is proposing, would then be a nice addition, but for me this would not be mandatory for this issue

[gregorriegler]

ok. but this would then be doeable by changing the wip-commit message to "!squash wip" and by following up with an rebase --autosquash before the mob done --merge right?

[koeberlue]

@gregorriegler thanks for pointing out autosquash. I didn't know this flag until know, very useful. Unfortunately I don't believe it will help us in this situation. If I understand correctly, starting a commit message with squash! will allow the rebase to squash the current commit with a previous commit. But I think we would want to mark the current commit to be squashed with the next "real" commit

[gregorriegler]

@koeberlue I also didn't know. @simonharrer told me about the autosquash thingy :)

[simonharrer]

Closing this for now, as v1.1.0 now has the mob done --no-squash flag.