I think that depends on the purposes for doing a specific work via a branch. If it's something you intend to put up for peer review via pull request, then this approach sounds very reasonable. That would be the kind of changes/additions where, in the past, we would have made patches that we posted on the forum for testing/review purposes.Dilvish wrote:The somewhat confusing situation Geoff related above seems to me to emphasize that even for us with commit writes to the main repository, it would be best to do most of our work via branches on our own forks, so on our local machines our individual github fork would be (normally) the 'origin' remote, and the main freeorion repository would be the 'upstream' remote. It seems to me that helps keep things organized and clean.
But I wouldn't use it otherwise. We didn't post patches for all things we committed, there were a lot of things we worked on in our local working copies we just committed when we thought them ready, for those we now would create local branches that we merge into our local master before pushing the changes.
And bigger projects like the SDL migration, revised colonization mechanics etc. where there might be several devs committing contributions can't be done with a branch in a "personal" fork anyway (unless you want to grant others push access to your personal fork, then you can do it, but I'd still fail to see the advantage of doing so in that specific scenario).
Well, from what I've read the opinions on the merging vs rebase topic seem to differ, with preferences for the one or the other being subject to personal taste, at least to some degree.It seems that one of the big benefits of going our individual work in feature branches that are either just on our local machines or (more commonly) also on our personal forks at GitHub, is that rebasing can be used to clean up the commit history before it gets pulled/merged/pushed to freeorion/freeorion, such as is described in this article.
Personally I agree with you, when rebasing is possible (that is, without screwing work others might have based on already published commit history), then it's preferable because it produces a more linear (less maze-like) history. So, with local only branches and branches you only pushed to your personal fork for pull requests, that sounds like a good approach. However, when it comes to squashing commits:
I think it's important to note that this approach should be used only within a certain (uh, which is the right word here?) context, or maybe conditions. Because I've also read recommendations to keep git commits "atomic", and not squeeze too many or unrelated changes into one commit. It makes isolating a "bad" (bug introducing) change easier (git bisect), ditto cherry-picking. As far as I understood, more but smaller commits are generally preferable to fewer but larger ones - as long as you don't go overboard of course.
By that I don't want to argue that there aren't cases where squashing commits makes sense. The example you quote is one of those cases where it obviously does. So, squashing commits is a good recommendation, as long as people take care not to go overboard with it. I wouldn't want someone to get the impression that it might be a good idea to squash all the commits he did on his local branch into a single huge one before merging into master. That's definitely not the idea.
Now, all that said, regarding using rebase on branches we put up for pull requests, I wonder if that makes any sense at all. Because, IIRC, and from what I've observed so far, it seems that merging a pull request via the github web interface always produces a merge with fast forward explicitely turned off. In that case a prior rebase is de facto pointless if I understood the whole mechanic correctly. What rebasing does is effectively changing the commit all the commits of your branch are based upon/derived from to the most recent commit of the branch you rebase onto. So that, when you do the subsequent merge, a fast foward merge can be done, resulting in a linear history that looks as if there has never been a branch.
If the subsequent merge command suppresses the fast forward feature, this "flattening" of commit history does not happen, so what would be the point of rebasing prior to the merge? So, in order to get the linear history we aim for, we would have to merge pull request manually instead of using the github web interface.
Did I understand that correctly, or am I missing something obvious? Marcel?