nest / nest-simulator

The NEST simulator
http://www.nest-simulator.org
GNU General Public License v2.0
544 stars 370 forks source link

Don't work on your master #31

Closed apeyser closed 8 years ago

apeyser commented 9 years ago

Working on your master leads to really nasty merge histories. You'll work, then merge into our master, and then continue working without pulling up to upstream, then remerging. You should keep your master pristine, work on a branch, and then do a pull request on the branch. After that, you should delete the branch and make a new branch from master.

This is something to check for in a code review --- that the history isn't crazy. If so, the author should be requested to rebase, otherwise eventually it'll become very hard to identify were bugs occurred and merges will become increasingly difficult to do correctly.

apeyser commented 9 years ago

You can check with gitk or at https://github.com/nest/nest-simulator/network. If the branch that is pull requested has already been merged into master... then reject.

gewaltig commented 9 years ago

"Working on your master leads to really nasty merge histories. You'll work, then merge into our master, and then continue working without pulling up to upstream, then remerging. " I am not sure I understand what you are saying here.

apeyser commented 9 years ago

Repositories:

If origin is a fork of upstream, and local is a clone of origin, then we have three masters:

upstream/master should only be changed via pull requests which are handled by the owners team. origin/master should be pushed to from local/master local/master should only pull --only-ff upstream master

and if you wish to work you should branch from local/master, push it to origin and then request that it be merged into upstream master via a pull request. At that point you could delete your local branch. When you update local/master from upstream master, it will be updated with what was your local branch, indirectly.

Otherwise, your local/master branch will diverge from upstream/master, and then your origin/master will diverge from upstream/master. You will end up really confused, others will end up really confused, the same patches will end up being applied multiple times through the branch history, and even worse things may happen.

naveau commented 9 years ago

@apeyser: is this picture illustrate your point? I was also not entirely sure to understand! git-workflow from http://fr.slideshare.net/rhwy/my-git-workflow

apeyser commented 9 years ago

Mikael, thanks, very nice. That is what you are supposed to do. If you work directly on your master, then step 7 would be a problem, and step 9. If your commits become interspersed on "ProjectA" with others, then you'd have to realize this and message your master by hand.

A number of developers have already had problems because of this, and I can see from the from the branching pattern that some more are also doing this and will eventually have problems, but if you follow the graphic, you have one way information flow which makes life easier.

gewaltig commented 9 years ago

Dear Alexander and Michael Thanks a lot for the clarification.

Best Marc-Oliver

Alexander Peyser notifications@github.com schrieb am Di., 16. Juni 2015 um 22:18:

Mikael, thanks, very nice. That is what you are supposed to do. If you work directly on your master, then step 7 would be a problem, and step 9. If your commits become interspersed on "ProjectA" with others, then you'd have to realize this and message your master by hand.

A number of developers have already had problems because of this, and I can see from the from the branching pattern that some more are also doing this and will eventually have problems, but if you follow the graphic, you have one way information flow which makes life easier.

— Reply to this email directly or view it on GitHub https://github.com/nest/nest-simulator/issues/31#issuecomment-112556298.

wschenck commented 9 years ago

I understand that interspersed commits become a problem when working on your own master and pulling from upstream (team account in the figure). But what would happen if you don't pull (which implies merge), but instead fetch from upstream and then rebase your master on upstream/master?

In my so far limited understanding of git, git should detect the double commits in upstream/master and your local master and remove the local ones?

Am 16.06.2015 22:18, schrieb Alexander Peyser:

Mikael, thanks, very nice. That is what you are supposed to do. If you work directly on your master, then step 7 would be a problem, and step

  1. If your commits become interspersed on "ProjectA" with others, then you'd have to realize this and message your master by hand.

A number of developers have already had problems because of this, and I can see from the from the branching pattern that some more are also doing this and will eventually have problems, but if you follow the graphic, you have one way information flow which makes life easier.

— Reply to this email directly or view it on GitHub https://github.com/nest/nest-simulator/issues/31#issuecomment-112556298.

Dr.-Ing. Wolfram Schenck SimLab Neuroscience Jülich Supercomputing Centre (JSC) Institute for Advanced Simulation Tel +49 2461 61-6719 www.fz-juelich.de/ias/jsc



Forschungszentrum Juelich GmbH 52425 Juelich Sitz der Gesellschaft: Juelich Eingetragen im Handelsregister des Amtsgerichts Dueren Nr. HR B 3498 Vorsitzender des Aufsichtsrats: MinDir Dr. Karl Eugen Huthmacher Geschaeftsfuehrung: Prof. Dr.-Ing. Wolfgang Marquardt (Vorsitzender), Karsten Beneke (stellv. Vorsitzender), Prof. Dr.-Ing. Harald Bolt,

Prof. Dr. Sebastian M. Schmidt


apeyser commented 9 years ago

Yes, if you were consistent you and understand rebase, you can do exactly what @wschenck suggests.

But why? Keeping your work on a separate branch and then rebasing the branch on master would give you the same effect with the additional advantage that git would then do extra bookkeeping for you, leading to a simpler and less error prone process.

heplesser commented 9 years ago

@apeyser, thanks for these explanations. I just have one question now. Assume I made changes in my branch fix1, pushed and created a pull request. It may take a little while before my PR is merged into the upstream master. In the meantime, I would like to be able to use fix1 in another branch of mine, feature2. How can I do that? Would something like

git checkout feature2
git merge fix1

work without causing problems later? I assume that my PR for feature2 would then also contain the commits included in the PR for fix1, and depending on the order in which PRs are merged, feature2 might even be merged into upstream/master before fix1. Will Git keep things tidy?

apeyser commented 9 years ago

If I get your question:

If you merge fix1 into feature2, then merging feature2 into master will merge fix1, thus you wouldn't make (or you'd cancel) the PR for fix1 when feature2 was merged. But I expect that if you're merging feature2, you'd merge fix1 into master first to minimize the size of each single merge.

So yes, history is global and if someone else had merged fix1 into feature3, then merging feature3 into master will not case a new set of fix1 patches to be introduced. Anything with the same hash is globally equal.

You could do even nicer things using options like rebasing, but for most people this is sufficient.

abigailm commented 8 years ago

@apeyser and @heplesser, what is required for this issue to be closed?

jougs commented 8 years ago

@abigailm: I think someone needs to review our documentation of the development workflow and check if the information in this issue is contained there. After that is the case, the issue can be closed.

sanjayankur31 commented 8 years ago

I can check the dev workflow if you'd like to assign this to me. :)

abigailm commented 8 years ago

@sanjayankur31 delighted to, thank you! :)

sanjayankur31 commented 8 years ago

It looks good! Some minor points/suggestions:

Cheers!

jougs commented 8 years ago

Dear @sanjayankur31, many thanks for the good work!

Regarding the use cases: they were mainly there to get things started for us and we were planning to remove them. Do you think we need them? If yes, we can also polish them instead.

Would it be possible to start a pull request containing your suggestions? The developer space pages are contained in the branch gh-pages of the nest-simulator repository, so the process is the same as it would be for changing code.

sanjayankur31 commented 8 years ago

Regarding the use cases: they were mainly there to get things started for us and we were planning to remove them. Do you think we need them? If yes, we can also polish them instead.

I think they can be removed then.

Would it be possible to start a pull request containing your suggestions? The developer space pages are contained in the branch gh-pages of the nest-simulator repository, so the process is the same as it would be for changing code.

Sure. I'll begin working on this now.