-
Notifications
You must be signed in to change notification settings - Fork 9
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
Making mgit close
really useful
#84
Comments
OK - I need to learn more about mgit since I'm using mostly That being said here are some thoughts on this.
|
I think that this command should be very similar to closing PRs on GitHub. It should let me edit the message, merge the branch and delete the branch. I agree that it should let me define the message it a nice way (opening the editor would be nice). Since we will use it often to merge all branches related to the PR it should also have some default message already prepared. I also agree that we need a confirmation question with all what |
Like for each merge in the constellation? Because usually, the messages are different across the packages in branch constellations. |
Many times, if you have N branches to merge, the message will repeat in N-1 or N-2 because there's one main change that gets a special message and most other repos get "Aligned to changes in ...". That's why I proposed that you can pick in which repos you want merge a change. |
I think that |
If it lets you to choose which repos you want to touch then it's up to you. You can deselect that main repo and close it the way you want. But personally, I usually close all from the console first and then push. Why? Because I can then make a quick check that I didn't screw something up. So, I'd do this:
And done. In fact, that'd be even better if mgit would work like this:
By letting me iteratively merge branches before steps 6 and 7, if anything gets bad I can easily revert everything. Perhaps even mgit could do that. |
This tool should also fetch all changes from the remote. It happens sometimes that someone else pushes some changes to the branch, but when I am closing this branch locally I forget about these changes. It is the most often when I am closing multiple branches. |
I agree with pulling changes to |
BTW, so I don't forget – one more reason for |
The discussion started in #82 (comment) and continued for a couple next comments.
Initially, I assumed that
mgit close
should work likegit merge
on steroids – you call it when on the branch to which you want to merge the given branch. But, in addition to callinggit merge
,mgit close
should also remove the branch from the remote and push the current branch to the remote.We started discussing whether
mgit close
should ask whether you want to remove the merged branch from the remote and whether to push the current branch to the remote too. I insisted on this because that:However, then I started realising that there's more about
mgit close
that we might improve. That it can be far more useful than a simple shorthand togit merge
:It can open a text editor so you can easily paste the merge message. This is very important because I still can't use backticks in the
-m ""
param, despite googling how to do that. Plus, multiline messages are very hard to type in the CLI.The workflow for closing branches is actually different than:
master
,xxx
tomaster
.When merging, you're usually on the setup of branches you want to close. There may be more branches called
xxx
in other repos, but you may not be using them at the moment. You are interested in closing exactly what you have there and just tested. Therefore, I want to propose a different workflow.Workflow
mgit sync
so to use the setup frommgit.json
.mgit close
(if executed without params it will merge tomaster
).master
.master
.master
before merging everything you callmgit push
.That's how my workflow with dealing with branch setups usually looks (but of course I do all those things manually). If
mgit close
worked this way I feel that it'd be most useful.Also, please bear in mind that we may still have
mgit merge
for moregit merge
-like behaviour. But we should see first if we need it.WDYT? cc @pjasiun @jodator @oleq
The text was updated successfully, but these errors were encountered: