Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mob start should start from the current branch instead of master #14

Closed
alexkli opened this issue Nov 21, 2019 · 23 comments
Closed

mob start should start from the current branch instead of master #14

alexkli opened this issue Nov 21, 2019 · 23 comments

Comments

@alexkli
Copy link

alexkli commented Nov 21, 2019

Current behavior

Currently, mob start will start the mob-session branch from master, regardless of what the currently checked out branch is. It is possible to control the base branch using MOB_BASE_BRANCH=mybranch mob start. However, this is non-intuitive and hard to remember.

Suggestion

mob start starts from the current checked out branch (if no mob-session exists and no local modifications, etc. same checks as before). You can still specify MOB_BASE_BRANCH to override that if you want.

The output would make it clear what branch was used and also tell the user how to undo, in case they did not want to start from the current branch, i.e. using mob done or mob reset.

Reasoning

Is there a particular reason or use case why it ignores the current branch and always starts mob-session based off of master?

In our experience, when you start the mob, you want to start it from your current branch. You might have already looked at the code before you actually run the mob start command, so there is IMO little chance that you want to start it from another branch (master). It also feels more natural to do

git checkout myfeature
mob start

than

MOB_BASE_BRANCH= myfeature mob start

Users will know git a lot better than the mob tool.

To given an example, we have been working on a feature branch that represents a more complex effort which we handle in the mob. This took many days and we don't necessarily do a mob every day. That means we split up some work in the meantime, contributed that back to the feature branch and the next day would do a mob and work on the integration. All the time the work focuses on that feature branch, not master.

OTOH, I feel that mob-session is not a feature branch, but just a "the mob is working on this today" temporary working branch, as there can only be 1, you don't really want to do a PR from mob-session and it's easy to loose the work using mob done or even mob reset.

@simonharrer
Copy link
Member

Thanks for the issue.

Is there a particular reason or use case why it ignores the current branch and always starts mob-session based off of master?

We, i.e. my team, only works in master. We don't use any other branch, except on exceptions. We found this is the best way when working full-time remote mob programming. I want to support this way.

I see your point in doing mob programming only for specific feature branches as you don't do mob programming full-time. Will have to think about the consequences in changing the current behaviour though. To work correctly, the information of the base branch would have to be stored somehow, making the handling more complex, because how does mob done know into which branch it should revert back?

I feel that mob-session is not a feature branch, but just a "the mob is working on this today" temporary working branch

That's how we think of it. May I add your description of the 'mob-session' branch to the README?

@alexkli
Copy link
Author

alexkli commented Nov 22, 2019

For mob start in your case, there shouldn’t be a problem afaics. If you only work against master then you will be on master when you start the mob. Or it should be no problem to make sure that’s where you are.

Regarding how would mob done know where it started from: that’s a good question.

It could try and infer that from the history, although that might be fragile if there were concurrent branch changes.

It could store this in a .mob file in the repo that would get removed as part of mob done (so that it always only exists on mob-session branches). Ideally you only add it upon mob next, so that just mob start doesn’t modify the local checkout yet. Here it could be inferred from the refs of the current revision at mob next time I think.

It could also just ask the user. In our case, when it comes to a mob done, everybody has to think twice anyway on where the result should go. So a prompt isn’t a bad idea because the user might not really know before they get to this point ;-)

Last but not least, since mob done doesn’t actually commit, but just stage, you can always adjust the branch at that point.

@OskarD
Copy link

OskarD commented Mar 13, 2020

mob start <original branch name(Default:master)>?

@Morl99
Copy link
Contributor

Morl99 commented Jun 8, 2020

We started experimenting with this cli, and I want to second this feature. As we are not full time mob programmers, we work with Merge Requests and feature branches. The current way imposes two inconveniences:

  1. You have to manually set the env variable before you start.
  2. Everyone on the mob has to do same.

@simonharrer
Copy link
Member

Thanks for joining in the discussion. I understand your points.

@simonharrer
Copy link
Member

How would this help:

master $ mob start
# switches to 'mob-session' branch as usual
mob-session $ mob next
# switches back to 'master' branch

feature1 $ mob start
# switches to 'mob-session-feature1' branch
mob-session-feature1 $ mob next
# switches back to 'feature1'  branch

The mob-session prefix in the branch allows the tool to easily detect whether a mob session is active or not. The only thing that is necessary: you need to create the 'feature1' branch before and make sure there's a remote branch 'origin/feature1'.

Would this help? Because this could be done with a reasonable amount of effort.

@simonharrer
Copy link
Member

I implemented it as a rough draft in #40 as described above.

Open issues:

  • 'mob reset' doesn't make sense with this feature enabled
  • what to do about the configuration options

Discarding the config options and the 'mob reset' command would be the easiest way forward.

@Morl99
Copy link
Contributor

Morl99 commented Jun 8, 2020

I like the idea of this, it also serves another purpose. If multiple mobs work on the same code base but on different branches, this would still work well.

Two thoughts:

  1. If you change the Prefix to mob-session/ a lot of tools allow to display this in a special folder like way.
  2. You could retrofit this approach to the master branch and just remove the special code for master by using mob-session/master. If you do, we would need to think about version upgrades though. Maybe doing this for the 1.0.0 version as a breaking change wouldn't be so bad though.

Mob reset could still delete the mob session branch though, right?

@simonharrer
Copy link
Member

Will use the '/' delimiter instead of '-'. Thanks for that!

I want to keep the tool backwards compatible. I would accept the few lines of extra logic for that.

Hm, yeah, I guess I can keep the mob reset method. Great.

@simonharrer
Copy link
Member

I added reset back, and used '/'.

Still thinking on renaming the prefix 'mob-session/' to 'mob/' to make it shorter.

@Morl99
Copy link
Contributor

Morl99 commented Jun 8, 2020

Shorter is good, I think it is highly unlikely, that there could be a conflict. And mob/feature-branch is still very obvious.

@alexkli
Copy link
Author

alexkli commented Jun 8, 2020

I like this approach!

Regarding conflict: the mob tool should never overwrite anything, e.g. delete and recreate a branch. IIUC, if a mob/something branch already exists, mob start would simply check it out, assuming it is joining the mob. Just needs to clearly state that in the output, so in case that branch did exist and was not a mob branch, folks will notice.

Not sure if there is any way for the tool to detect that a branch is a "mob" branch or not. If that's deemed necessary, maybe it could add a special marker to its commit messages, and if the most recent commit doesn't have it, it could log a bigger warning or something like that.

@Morl99
Copy link
Contributor

Morl99 commented Jun 8, 2020

I wouldn't like that. We frequently push to the mob branch during sessions to trigger our CI Pipeline, so that is not a handover, but part of the work. This is especially needed if the work is improvement or enhancement of the pipeline itself. The CLI should not mind other pushes, and why would it? It won't destroy any work anyways.

@alexkli
Copy link
Author

alexkli commented Jun 9, 2020

Makes sense. I was just thinking If that's deemed necessary, but we wouldn't have a requirement for it to detect and handle it differently.

@micha149
Copy link

micha149 commented Jun 9, 2020

We just started to give mob a try and came across exactly this problem ^^ Looking forward to this fix!

@simonharrer
Copy link
Member

Will be part of the next release

@nlochschmidt
Copy link

@simonharrer This is awesome, but looks like the documentation on https://mob.sh is now outdated, right?

FYI: I got the new release 0.0.20 through homebrew and was surprised by this change.

@simonharrer simonharrer mentioned this issue Jun 18, 2020
@simonharrer
Copy link
Member

Thanks for your feedback @nlochschmidt

I try to keep the documentation up to date. This particular feature, however, is missing on the docs. Thanks for pointing that out. I've created #48 for that.

Is you being suprised good or bad in that context? Anything I can to help to make it better? Would a version bump of the major or minor version number have helped? Is the CHANGELOG not clear enough?

@nlochschmidt
Copy link

@simonharrer just a perfect storm of multiple things:

  1. In a recent mob session I was the only one using 0.0.20 while the others where still on 0.0.18
  2. My specific zsh prompt setup is swallowing xyz/ prefixes in the branch names so I didn't see that I am on mob/branch instead of just branch

I don't think a version bump would have helped. I am not even aware if it's possible to version-lock brew installed tools. I typically just want them to upgrade to latest anyways.

The changelog so far is something that needs to be opened up manually and I only looked on https://mob.sh. So it would definitely be nice to have the latest breaking changes to the tool visible on the main site.

However ultimately, I think it would have helped most, when an error would be raised when setting the MOB_BASE_BRANCH and MOB_WIP_BRANCH env variables, because afaik they are no longer respected by mob.

@simonharrer
Copy link
Member

I see. First of all, I'm sorry that you encountered such a bad experience with 'mob'. I honestly haven't thought that anyone actually overrides MOB_BASE_BRANCH and MOB_WIP_BRANCH. We don't use them in my team at all... and I honestly haven't thought that 'mob' is used by so many people... I try to keep that in mind when moving the tool forward.

I've created #50 so a warning will be shown when using removed config options.

Not sure whether I want to maintain a changelog in CHANGELOG.md and README.md as it's already quite a hassle to maintain the tool alone next to the day job and my family...

@simonharrer
Copy link
Member

@nlochschmidt we've just implemented 8adf25d

Does this feature support your use case?

Regardless, we plan to implement #50 nevertheless.

@nlochschmidt
Copy link

Yes! At least I think so 🙂

Thanks a lot for all the work you put into this, even though it's not required for your own use-case 👍

@simonharrer
Copy link
Member

Release including #50 is now available. Happy remote mob programming! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants