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

Redesign the set of mgit commands #73

Closed
Reinmar opened this issue Apr 11, 2018 · 35 comments · Fixed by #82
Closed

Redesign the set of mgit commands #73

Reinmar opened this issue Apr 11, 2018 · 35 comments · Fixed by #82

Comments

@Reinmar
Copy link
Member

Reinmar commented Apr 11, 2018

Right now we have a pretty messy situation where we have mgit bootstrap, mgit update, mgit co. The first two are quite similar, but none actually follows git pull and git fetch.

The initial idea was to follow Lerna's naming. But after a time we see that mgit should follow git's naming. Ideally, pretty much 1:1.

So, it could look like:

  • mgit pull – does what git pull does + clones missing repos
  • mgit co – checks out to branches listed in mgit.json
  • mgit co <branch> – checks out all repos to the given branch
  • mgit co -b <branch> – creates new branches (in all repos in which you have changes?)
  • mgit fetch – does what git fetch dies (+ clones missing repos?)
  • mgit push – does what git push does
  • mgit commit <params...> – does what git commit does
  • mgit save --branches – saves current configuration of branches to mgit.json
  • mgit save --hashes – saves hashes to mgit.json
  • mgit st – like today
  • mgit diff – like today
  • mgit exec – like today
  • mgit merge – to simplify closing multi-repo PRs

However, what we need to understand is whether these commands do match our workflow. I mean here the old mgit update which was actually a sum of the new mgit co && mgit pull. Are we ok with the fact that now you'll have to think more about what branches you're on and that mgit pull is not all if you were on some completely different setup previously?

@jodator
Copy link
Contributor

jodator commented Apr 11, 2018

From the above I'd like to see (as I'm missing those tasks):

  • mgit save --branches
  • mgit co -b <branch> but only if it will "creates new branches (in all repos in which you have changes" - I've got mass branch creation in IDE
  • mgit pull with "+ clones missing repos"

The other are either meh (for me) or I have similar functionality in the IDE - like mgit co <branc> or mgit commit or mgit push - all are nicely supported by the Web/PHPStorm.

@oleq
Copy link
Member

oleq commented Apr 11, 2018

mgit commit <params...> – does what git commit does

Would it commit in all packages or just the ones which are different than mgit.json, or... what?

@adelura
Copy link

adelura commented Apr 11, 2018

All of those sounds very reasonable for me, I think that it's a good approach to think about this tool as multi git rather than lerna tool. It can be used with lerna easily but it can be transparent about it.

Currently I'm using mgit just for bootstrap command, I will be pleased when I see commands like, commit, push, checkout and even stash.

====

To automate the workflow even more I would like see that my hashes in mgit.json are updated automatically when I execute mgit commit (for all nested repositories) and on root repository it will commit my mgit file as well.

@Reinmar
Copy link
Member Author

Reinmar commented Apr 11, 2018

To automate the workflow even more I would like see that my hashes in mgit.json are updated automatically when I execute mgit commit (for all nested repositories) and on root repository it will commit my mgit file as well.

Interesting idea. Mgit could automatically update hashes if it had hashes stored in mgit.json previously. And could save there branches if you... used branches.

The tricky thing is – can it know when to do that and when not? I'm often jumping between repos and branches in these repos and I don't want mgit to constantly change my mgit.json.

@Reinmar
Copy link
Member Author

Reinmar commented Apr 11, 2018

Would it commit in all packages or just the ones which are different than mgit.json, or... what?

All changes that you've done. A shorthand for mgit exec 'git commit -a -m ...'?

Which reminded me about mgit merge :D I'll add it to the list.

@pomek pomek self-assigned this Apr 11, 2018
@ma2ciek
Copy link
Contributor

ma2ciek commented Apr 11, 2018

Interesting idea. Mgit could automatically update hashes if it had hashes stored in mgit.json previously. And could save there branches if you... used branches.

The tricky thing is – can it know when to do that and when not? I'm often jumping between repos and branches in these repos and I don't want mgit to constantly change my mgit.json.

Maybe we can provide some flag to explicitly save branches, hashes or don't commit changes in the main repo at all?
Like mgit commit --update-main-repo=hashes ...

@Reinmar
Copy link
Member Author

Reinmar commented Apr 11, 2018

Would it work like a watcher? Or would it change some property in mgit.json? And what if you'd do:

cd packages/ckeditor5-engine
git co t/foo

@adelura
Copy link

adelura commented Apr 11, 2018

Which reminded me about mgit merge :D I'll add it to the list.

Don't forget about mgit rebase -i 😎

I'm often jumping between repos and branches in these repos and I don't want mgit to constantly change my mgit.json.

This is where the "watch mode" comes into action and convenient saving state will help. Generally speaking - if developer is going to do some "illegal" action like you mentioned above:

cd packages/ckeditor5-engine
git co t/foo

I would warn user with message like: "Hey men! You are trying to checkout|commit|rebase directly into 'ckeditor5-engine' repository. Do you want to save your changes first?"

====

The other are either meh (for me) or I have similar functionality in the IDE - like mgit co or mgit commit or mgit push - all are nicely supported by the Web/PHPStorm.

@jodator How do you commit to multiple repositories? Do you use some plugin?

<probably-offtopic>

By the way also I found it annoying when I checkout hashes, so I lost track of branch to which I want to commit to. Not sure, how your workflow looks like, but did you had similar issues?

There might be possibility to keep track of both - branches and hashes in mgit.json. As far as I remember current syntax is:

{ "repo-name": "user/repo-name#d4da809"}

it might be resolved by storing both:

{ "repo-name": "user/repo-name#master#d4da809"}

It might be helpful when you want to commit changes.

</probably-offtopic>

@Reinmar
Copy link
Member Author

Reinmar commented Apr 12, 2018

I would warn user with message like: "Hey men! You are trying to checkout|commit|rebase directly into 'ckeditor5-engine' repository. Do you want to save your changes first?"

But how? You would need a githook inside that package. While this is certainly possible, that's quite a lot of work.

By the way also I found it annoying when I checkout hashes, so I lost track of branch to which I want to commit to. Not sure, how your workflow looks like, but did you had similar issues?

If you check out to hashes, then I don't think it's not common to try to do a commit in that state. It's like git co <hash> – it leaves you at a detached state. Like here:

image

If mgit would save hash+branch, then when checking out to that sate it would actually have to do this:

git co <branch>
git reset --hard <hash>

Which would allow committing, but not pushing these changes (if you were behind the origin). So I think that the only reasonable solution is to save only branches in mgit.json. Mgit should not interfere with typical gitology.

@jodator
Copy link
Contributor

jodator commented Apr 12, 2018

@jodator How do you commit to multiple repositories? Do you use some plugin?

@adelura: If you open CKEditor repository with all sub-repos in ./packages the *Storm IDE will ask you to add unregistered git roots. You can also do it in version control settings. When you have more then one VCS registered then *Storm commands like commit, push, update project will run on all those registered commands.

ps.: It's built in functionality (I'm on 2018.1.1 now, but it works at least in 2017.x versions). But I can see the blog post from 2012 mentioning this also.

@pomek
Copy link
Member

pomek commented Apr 12, 2018

Don't forget about mgit rebase -i 😎

Guys, take into consideration that mgit cannot have interactive commands because it's running in parallel threads.

@pomek
Copy link
Member

pomek commented Apr 12, 2018

mgit exec 'git commit -a -m ...'

git commit -a ... is a bad command and never should be used because you could commit something that you shouldn't.

We should manually go through all repos and add files to commit manually. Then use mgit commit -- -m "Message" which will execute git commit -m "Message" in all repos.

@Reinmar
Copy link
Member Author

Reinmar commented Apr 12, 2018

git commit -a ... is a bad command and never should be used because you could commit something that you shouldn't.

Not if you'll do mgit diff before. And I often do exactly that. mgit diff + mgit exec 'git commit ...'. The tool should not assume that we don't know what we do.

@pomek
Copy link
Member

pomek commented Apr 15, 2018

The tool should not assume that we don't know what we do.

Mhm. Ok.


Let's summary the whole talk about new commands:

mgit pull – does what git pull does + clones missing repos

Could it just pull the changes? We could have mgit clone command to clone missing repos and mgit pull to pull the changes. I would like to have "one command which does one thing".

In the future mgit clone <repo-url> could clone given repository and save them to the mgit.json using its name from package.json.


mgit co – checks out to branches listed in mgit.json
mgit co <branch> – checks out all repos to the given branch
mgit co -b <branch> – creates new branches (in all repos in which you have changes?)

Everything sounds good except that parameters for the command must be separated by --, so it will be:

mgit co -- -b <branch>

It must be done this way because we can specify the scope for mgit, e.g.:

mgit co --scope=*build* -- -b t/some-ticket-id

mgit fetch – does what git fetch dies (+ clones missing repos?)

I would like to remove cloning repos and introduce --prune (-p) flag which:

Before fetching, removes any remote-tracking references that no longer exist on the remote.


mgit push – does what git push does

👍


mgit commit <params...> – does what git commit does

mgit commit -- [params]

The [params] block will be passed to git command. --all flag will be addded automatically, so git will call for each repository:

git commit --all [params]

Is it good?


mgit save --branches – saves current configuration of branches to mgit.json
mgit save --hashes – saves hashes to mgit.json

Could it be: mgit save branches and mgit save hashes?


mgit st – like today
mgit diff – like today
mgit exec – like today

👍


mgit merge – to simplify closing multi-repo PRs

We could introduce the --message option which allows specifying the merge message. It can be usefull when we would like to save reference to the ticket.

@jodator
Copy link
Contributor

jodator commented Apr 16, 2018

mgit co – checks out all repos to the given branch
mgit co -b – creates new branches (in all repos in which you have changes?)

Just a reminder - that it would be helpful to create branches only on modified repos.

As for checkout - should it only checkout existing? (no create)

@pomek
Copy link
Member

pomek commented Apr 16, 2018

only on modified repos

We could introduce an option for the whole mgit: --modified or as an alias -m. It will be a complement option for the rest option which changes the scope (--scope or --ignore).

It should execute given command only for repos that contain some changes. But we should define what it does mean? Is it branch other than specified in mgit.json? Or is it the repo where git st -s does not return empty response?

@pomek
Copy link
Member

pomek commented Apr 16, 2018

The -m option could be useful for other commands like push, merge, diff or even save --hashes.

@jodator
Copy link
Contributor

jodator commented Apr 16, 2018

It should execute given command only for repos that contain some changes. But we should define what it does mean? Is it branch other than specified in mgit.json?

Probably command specific. As one example
mgit co -b branch -m - I'd like to checkout all modified packages to a specified branch
mgit co -m - probably makes no sense
mgit save --branches -m - save current branches but only from modified packages (does it makes sense?)

I wonder if -m makes sense for other commands?

@pomek
Copy link
Member

pomek commented May 11, 2018

I'm wondering what I can say about the whole topic as a summary and I don't really know. I have a feeling that the whole topic is about adding new commands or changing the name of existing ones.

The final proposition about the set of mgit commands:

  • pull – the same what git does (make sense if you're on master and want just to pull all the things),
  • push – the same what git does,
  • fetch – the same what git does (it could accept the --prune flag),
  • merge – the same what git does. It will work only if the current branch is other than master or specified in the mgit.json. It also could accept the --message option. If it won't be specified, the merge message will be created by git.
  • checkout – the same what git does, (options will be described below),
  • bootstrap – the same what doe now (it doesn't require any change so leave it without changes),
  • diff – the same what does now,
  • status (st) – the same whas does now,
  • exec – the same what doe now,
  • commit – an alias for git commit -a. It will require a --message option.
  • save – by default --hashes but it can also be --branches. It will save the hashes or branches to the mgit.json (savehashes will be renamed),
  • sync - an alias for four commands: (fetch && checkout && pull) || bootstrap (at this moment mgit update does the same).

checkout

By default it will checkout the project to the state saved in mgit.json.

But it also can be used to create a new branch or checkout to existing branch:

  • mgit checkout master will checkout each repository to #master branch,
  • mgit checkout t/ckeditor5/1000 --modified will checkout (or create) a branch #t/ckeditor5/1000 in repositories that git status -s returns non-empty string.

The --modified option changes the scope of mgit so it isn't attached to the command option (like in diff – mgit diff -- [option for diff command])

sync

The goal is to have a command which updated the whole projects. We could use mgit bootstrap && mgit fetch && mgit checkout && mgit pull but it isn't comfortable.

In the details, sync will call the fetch, checkout and pull command for existing repo and bootstrap for non-existing.

Let me know guys WDYT because would be good to start implementing these commands.

@Reinmar
Copy link
Member Author

Reinmar commented May 15, 2018

mgit checkout t/ckeditor5/1000 --modified will checkout (or create) a branch #t/ckeditor5/1000 in repositories that git status -s returns non-empty string.

I'm unsure about this. Can it be just mgit co -b <branch name>? Normally, git co -b <branch name> always creates a new branch and checks out to it. mgit co -b could be smarter and do this only in repos which have any changes.

The goal is to have a command which updated the whole projects. We could use mgit bootstrap && mgit fetch && mgit checkout && mgit pull but it isn't comfortable.

Doing git fetch and git pull will slow down this command. The most time-consuming task is the communication with the remote server. Ideally, the task should do something like git fetch && git co <branch name> && git reset --hard origin/<branch name>. Or something safer than git reset --hard but still something which avoids hitting the network bottleneck twice.

Please also do remember that all commands which may lead to losing some uncommitted changes should abort if the repo isn't clean.


As for the rest, all sounds fine to me.

@pjasiun
Copy link

pjasiun commented May 15, 2018

sync - an alias for four commands: (fetch && checkout && pull) || bootstrap (at this moment mgit update does the same).

It does not make sense to do pull if you already did fetch. pull = fetch+ merge so you can just do merge.

But what I prefer to do for sync is reset --hard instead of merge.

@scofalik
Copy link

👍 from me.

@Reinmar
Copy link
Member Author

Reinmar commented May 15, 2018

One more thing – it'd be great if doing this:

mgit merge t/xxx -m "Fix: Whatever."

Executed actually this:

git merge --no-ff t/xxx -m "Merge branch 't/xxx'" -m "Fix: Whatever."

So, I think that we can safely assume for now that --no-ff should be added. And I'd also like if the first default line "Merge branch 't/xxx'" was added automatically so we only need to write the "Fix: Whatever" when calling mgit. Just like on GH when merging PRs where you only focus on the message (the textarea), not the title (the first input):

image

@ma2ciek
Copy link
Contributor

ma2ciek commented May 15, 2018

IMO it's confusing that some mgit commands will match 1:1 and some won't. Providing new ones as a shortcut for few others is for me ok, but not breaking old GH ones.

@Reinmar
Copy link
Member Author

Reinmar commented May 15, 2018

How would you see them work then, if the most important requirement is that mgit should speed up our daily activities? What about creating new branches only in repos with changes? Or making mgit merge easier to use than git exec 'git merge --no-ff t/xxx -m "Merge whatever" -m "Real message." which gets extremely long and I never remember what to write there?

@Reinmar
Copy link
Member Author

Reinmar commented May 15, 2018

Please also do remember that all commands which may lead to losing some uncommitted changes should abort if the repo isn't clean

Also, mgit sync should abort when we're ahead of the origin because git reset --hard will lead to losing these changes. That's why it'd be great to find something safer than git reset --hard. Something which will make mgit sync behave like git pull (which rebases local changes).

@pjasiun
Copy link

pjasiun commented May 15, 2018

behave like git pull (which rebases local changes).

To be precise git pull will merge, not rebase, by default.

@Reinmar
Copy link
Member Author

Reinmar commented May 15, 2018

I think it depends on this (which I think all of us should have in our .gitconfigs)

[branch]
	autosetuprebase = always

This option should be on by default. I don't like the fact that git merge's things when you pull changes. This makes the history more complicated than it needs to be.

@pjasiun
Copy link

pjasiun commented May 15, 2018

You can also use pull --rebase. I have a purr alias for it and just use it.

@ma2ciek
Copy link
Contributor

ma2ciek commented May 15, 2018

Maybe we can allow making aliases in mgit, so everyone will be able to create a set of commands that he uses.

E.g mgit purr can be an alias to mgit exec "git pull --rebase".

@Reinmar
Copy link
Member Author

Reinmar commented May 15, 2018

Maybe we can allow making aliases in mgit, so everyone will be able to create a set of commands that he uses.

The problem is that a command like mgit sync is not a simple "do x && do yy". It needs to check some things, do some things in all repos, do them again, based on info from these repos.

Besides, DRY. If all of us will have to create aliases, then something's wrong.

@pjasiun
Copy link

pjasiun commented May 15, 2018

Or, maybe mgit could get custom aliases and let use them as comments which will execute on all repos? Of course, as long as there are no conflicts with mgit custom commands. Each of us, use a different set of commands, but it is possible to get all aliases on one's git (git config --get-regexp alias).

I mostly use my aliases, some of them are pretty complex, use sh, and I would most probably do not use the set you propose.

@Reinmar
Copy link
Member Author

Reinmar commented May 15, 2018

IMO, you're overengineering this. Suddenly, we're going to parse existing aliases and force all of us to setup them just to avoid.. doing what? Preconfiguring git commands?

I don't like the fact that we'll use the same names as git, but to do slightly more specific way (please note – we're not doing something different; we'll just preconfigure them!). But that's not a reason to introduce all these complications.

Besides, currently mgit co does something else than git co and what? Did you ever have a problem with that? I don't think so.

@pjasiun
Copy link

pjasiun commented May 15, 2018

I also think that command which does not do exactly the same should be called differently. For instance, I would expect that mgit commit will do git commit (with no -a flag). We may have mgit ca for git commit -a. The same for mgit merge. We may have mgit close for closing PRs since it does something different than git merge.

@Reinmar
Copy link
Member Author

Reinmar commented May 15, 2018

Sounds good to me. I'm only unsure about mgit close because closing PRs does not mean merging them. Can we find some other name? If not, then I'd keep mgit merge. --no-ff is not an uncommon addition anyway.

Reinmar added a commit that referenced this issue Jan 11, 2019
Feature: Introduced a set of new commands which should help developers in daily tasks. Closes #73.

*  New commands:

    * `commit` - allows committing all changes files that are tracked by Git (a shorthand for `mgit exec 'git commit -a'`)
    * `fetch` - allows fetching changes in all cloned repositories (a shorthand for `mgit exec 'git fetch'`)
    * `pull`  - allows pulling changes in all cloned repositories and cloning missing ones (it does not check out to specified branch in `mgit.json` file)
    * `push` - allows pushing changes in all cloned repositories (a shorthand for `mgit exec 'git push'`)
    * `close` - allows mering specified branch into current one and removes the merged branch from the local and remote

* The `update` command was renamed to `sync`.

* The `save-hashes` command was renamed to `save`. It accepts two options: `--branch` or `--hash` (which is default one). If specified `--branch`, name of current branches will be saved in `mgit.json`.

* Removed command `bootstrap`. Use the `sync` command instead. Sync command will scan the package directories and compare results with packages saved in configuration file. If there is something that is not defined in `mgit.json`, it will be printed out.

* `checkout` command now allows checking out the project to specified branch: `mgit checkout stable` will check out all repositories to `#stable` branch. It can also create a new branch for repositories that contains changes in files tracked by git. Calling `mgit checkout -- --branch develop` will create the `#develop` branch in these repositories.

* Improved the help screen of mgit and introduced a help screen for specified command, e.g.: `mgit sync --help`.

BREAKING CHANGE: Removed the `bootstrap` command. The `sync` command should be used instead for initializing the repositories.

BREAKING CHANGE: Renamed `update` command to `sync`.

BREAKING CHANGE: Renamed `save-hashes` command to `save`. It supports two parameters: `--branch` and `--hash` which the second one is set as default.

NOTE: `mgit checkout branch` will check out the repository on `#branch`. `[branch]` argument is optional. If it isn't specified, branch name will be taken from `mgit.json`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants