FreeOrion

Forums for the FreeOrion project
It is currently Wed Dec 13, 2017 3:22 am

All times are UTC




Post new topic Reply to topic  [ 188 posts ]  Go to page Previous  1 ... 8, 9, 10, 11, 12, 13  Next
Author Message
PostPosted: Sun Apr 10, 2016 6:57 pm 
Offline
Programmer
User avatar

Joined: Fri Mar 01, 2013 9:52 am
Posts: 1040
Location: Germany
Vezzra wrote:
Do you think that will work/is a sound policy?


No, because the same reasons mentioned again and again still apply regardless what's the intent of the PR and I'm too lazy to reiterate them again.

_________________
Resident code gremlin
Attached patches are released under GPL 2.0 or later.
Git author: Marcel Metz


Top
 Profile  
 
PostPosted: Thu Apr 14, 2016 10:01 pm 
Offline
AI Contributor
User avatar

Joined: Tue Jun 24, 2014 9:55 pm
Posts: 444
Suggesting for one commit branches.

1) at pull request creation author adds label :SquashAndMerge:
2) Author rebase master if necessary, but does not squash/edit commits manually. I want to check what was done after review, not whole diff as single commit. It really saves my time.
3) User who close this PR will to choose SquashAndRebase and edit commit message

_________________
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0


Top
 Profile  
 
PostPosted: Fri Apr 15, 2016 1:16 pm 
Offline
Release Manager, Design
User avatar

Joined: Wed Nov 16, 2011 12:56 pm
Posts: 4288
Location: Sol III
adrian_broher wrote:
No, because the same reasons mentioned again and again still apply regardless what's the intent of the PR and I'm too lazy to reiterate them again.
Well, I didn't ask for a reiteration of all the reasons why merging master into PR branches is a bad idea. Actually all I wanted to know was if merging master into a feature branch right before merging that feature branch back into master could be a way to address these issues. Judging by your answer, apparently not.

However, Cj's wish to keep the Fighters feature branch up to date with master isn't unreasonable, so how can that be achieved? Frequently rebasing the feature branch obviously also has some issues (we're talking about a feature branch several people will collaborate on, not a PR branch basically only one person works with), what other alternatives are there? Just stating that a certain approach is a bad idea isn't enough, we need to offer a usable alternative.

So my questions are: Is it really impossible to do merges of master into a feature branch in a way that won't result in a big mess once the feature branch gets merged back into master? Or rather, @Cj, is it really that impossible to come up with a workflow that can work sufficiently well with frequent rebasing of the Fighters feature branch (instead of merging master)? I suppose the people working together on that branch would just have to coordinate carefully: each rebase needs to be announced, so all contributors can merge what is already ready, and bring their local checkouts into a state where they also can be easily rebased afterwards... shouldn't be too hard, no?

Please, guys, we need to come to some consensus here (and last but not least, someone needs to make a final decision - I'm not sure if I'm the right one for that, however... Geoff? Sorry if I get a bit annoying sometimes, but you're the programming lead after all...).


Top
 Profile  
 
PostPosted: Fri Apr 15, 2016 1:20 pm 
Offline
Release Manager, Design
User avatar

Joined: Wed Nov 16, 2011 12:56 pm
Posts: 4288
Location: Sol III
Cjkjvfnby wrote:
Suggesting for one commit branches...
Sounds reasonable. Anyone any objections against/comments on adopting that policy?


Top
 Profile  
 
PostPosted: Fri Apr 15, 2016 2:45 pm 
Offline
Programming, Design, Admin
User avatar

Joined: Wed Oct 08, 2003 1:33 am
Posts: 12041
Location: Munich
Vezzra wrote:
Please, guys, we need to come to some consensus here (and last but not least, someone needs to make a final decision - I'm not sure if I'm the right one for that, however... Geoff? Sorry if I get a bit annoying sometimes, but you're the programming lead after all...).
I don't understand the problem with periodically rebasing a branch on master... The merging conflicts will need to be dealt with eventually either way. Keeping them as up to date as possible is reasonable. If you're worried about rebasing causing problems with others local changes, perhaps schedule a regular time for the rebasing against master?

I don't really know what best practice for this sort of issue is, though; I'd likely defer to adrian broher or someone else with more experience and competence with git / github.


Top
 Profile  
 
PostPosted: Fri Apr 15, 2016 5:46 pm 
Offline
Creative Contributor
User avatar

Joined: Fri Jun 28, 2013 11:45 pm
Posts: 3291
It was my understanding Adrian had already said to rebase the feature branch every so often, but to do so with warnings to other contributors in case of potential issues.

I've been busier than normal this week and not been looking at things but was intending to rebase it this weekend assuming nothing went wrong. I dislike merges generally, and much prefer rebased branches upon merge, ideally with as many superflous commits squashed as possible.

_________________
Mat Bowles

Any code or patches in anything posted here is released under the CC and GPL licences in use for the FO project.


Top
 Profile  
 
PostPosted: Fri Apr 15, 2016 8:47 pm 
Offline
Programming, Design, Admin
User avatar

Joined: Wed Oct 08, 2003 1:33 am
Posts: 12041
Location: Munich
What happens if you merge master into a branch a few times, then at the end before merging back, do a rebase against master?


Top
 Profile  
 
PostPosted: Sat Apr 16, 2016 9:34 am 
Offline
AI Contributor
User avatar

Joined: Tue Jun 24, 2014 9:55 pm
Posts: 444
Geoff the Medio wrote:
What happens if you merge master into a branch a few times, then at the end before merging back, do a rebase against master?


Extra work with place for human error on conflict resolve.

_________________
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0


Top
 Profile  
 
PostPosted: Sun Apr 17, 2016 8:22 am 
Offline
Programmer
User avatar

Joined: Fri Mar 01, 2013 9:52 am
Posts: 1040
Location: Germany
Sorry for the late response, fever and headache struck me down again the last days.

Vezzra wrote:
However, Cj's wish to keep the Fighters feature branch up to date with master isn't unreasonable, so how can that be achieved? Frequently rebasing the feature branch obviously also has some issues (we're talking about a feature branch several people will collaborate on, not a PR branch basically only one person works with), what other alternatives are there? Just stating that a certain approach is a bad idea isn't enough, we need to offer a usable alternative.


I don't understand why do you want to keep the feature branch up-to-date by any means. I can only come up with two reasons beside the obvious I want the feature branch up to date because I like things up to date:

  • master or <other branch> provides a prerequisite the feature branch needs, may it be a bug fix, a new interface, …
  • An external contributor added an PR to master, which provides features only intended for feature branch and this PR was already merged.

The first case should be solved with `git cherry-pick` by picking the commits that are needed on the feature branch from the source branch. When rebasing before the final merge git will sort those already existing commits out, leaving only feature related commits.

The second case is just a special variant of the first case, except we screwed up multiple times in terms of workflow. We didn't say the contributor that he should base his PR against the feature branch and we merged the wrongly based PR anyway. To fix this you can also use cherry-picking.

_________________
Resident code gremlin
Attached patches are released under GPL 2.0 or later.
Git author: Marcel Metz


Top
 Profile  
 
PostPosted: Sun Apr 17, 2016 1:53 pm 
Offline
Release Manager, Design
User avatar

Joined: Wed Nov 16, 2011 12:56 pm
Posts: 4288
Location: Sol III
Geoff the Medio wrote:
I don't understand the problem with periodically rebasing a branch on master
It increases administrative overhead, so to speak. You have coordinate more carefully. If I rebase a branch which others have checked out locally, their local branches become more or less invalid, because the commits in those branches no longer are diffs against the same base. If your local branch contains commits you haven't pushed before the remote branch got rebased, you can't just rebase those "extra" commits on the remote branch, because in case you try that, your whole local branch (extra commits and those which are now "duplicates" of the rebased remote branch) will get rebased on the remote branch, which obviously will end in a big mess.

AFAIK the best way to clean that up in case you find yourself in this situation (having commits you haven't pushed to a branch that got rebased), is to check out a new local copy of the remote branch and cherry pick your commits from the "old" local branch to the new one (Marcel, did I get that right?). No problem if it's only a few commits, but if you've been working on something bigger, not so funny.

To avoid the above scenario, you need to announce a planned rebase and give the involved contributors enough time to wrap up and push whatever they have sitting in their local branches. Once everyone has pushed everything, you can rebase the branch, and everyone can proceed by just deleting their old local copies (as there are no more local changes that would get lost) and make a fresh check out of the rebased branch.

However, during that time you shouldn't of course start on something new locally, but need to wait until the rebase has been done, and then you can resume with whatever you've been working on the feature branch. That's no insurmountable problem, it's just a bit more work in the coordination department, and requires the participants to be sufficiently self-disciplined...
Quote:
I don't really know what best practice for this sort of issue is, though; I'd likely defer to adrian broher or someone else with more experience and competence with git / github.
That's perfectly fine for me. Having Marcel, who is the most experienced with git/github, in charge of these policies sounds very reasonable. I just wanted to have things clearly stated. So, if no one objects, I'm all for putting him in charge of this.

That doesn't mean I won't argue with him on certain points, like I'm just doing here. ;) But the final call shall be his. :)


Top
 Profile  
 
PostPosted: Sun Apr 17, 2016 2:44 pm 
Offline
Release Manager, Design
User avatar

Joined: Wed Nov 16, 2011 12:56 pm
Posts: 4288
Location: Sol III
adrian_broher wrote:
Sorry for the late response, fever and headache struck me down again the last days.
No problem. I'm sitting on a Mt. Everest sized mountain of topics I didn't replied to immediately but still want/wanted to once I find the time to get back to them. The posts/topics at the bottom of that pile can safely be classified as prehistoric by now, so who am I to complain... ;)

Hope you're reasonably well now.
Quote:
I don't understand why do you want to keep the feature branch up-to-date by any means.
For a feature branch that won't be around very long and will be merged fairly quickly, no problem. But in my experience, once you've got a long running feature branch, not keeping it up to date and trying to resolve all conflicts when doing the final merge back into master is a recipe for headache. The longer a branch is out of sync with master, the more conflicts accumulate over time, and resolving all of them in one big cleanup is a tedious, mind-numbing and error prone task.

If, on the other hand, you've frequently incorporated the changes in master into your branch, conflict resolving is easier and you don't have to clean up a (potential) mountain at the time of the final merge.
Quote:
master or <other branch> provides a prerequisite the feature branch needs, may it be a bug fix, a new interface
That could work, if we are only dealing with the occasional prerequisite/bug we need to integrate, but that's not what I expect with a feature branch like the one in question here.

First of all, you'll probably want to integrate all bugs that get fixed in master, and as fixed bugs aren't exactly a rare occasion, that's already a fair amount of cherry-picking you'd have to do.

Second, we probably not only want prerequisites in the strict sense (things which the feature branch must integrate to work at all), but all things/features added/changed in master that will in one way or another interact with the feature that gets worked on in the feature branch. Example: if the new supply propagation mechanics had been pushed to master after the Fighters branch got created, we most likely would have wanted to intgrate that into the Fighters branch. It wouldn't have been absolutely necessary, as you could have worked on the fighter and carrier mechanics with the old supply propagation mechanics as well, but that wouldn't be very reasonable IMO. Those mechanics interact, and of course it would be far better to adjust the fighter and carrier to the revised supply propagation mechanics.

However, cherry picking an entire feature branch that got merged into master sounds awfully tedious. Besides, there are probably a lot of other changes you'd want to integrate into the Fighters branch, because they might impact how you implement/balance/adjust things with fighters and carriers.

All of which will lead to an awful lot of cherry picking in the end - at least, that's what I expect will happen. And for that, frequent rebasing (if we don't want to merge master) seems to be the better approach than cherry picking in my eyes.
Quote:
The first case should be solved with `git cherry-pick` by picking the commits that are needed on the feature branch from the source branch. When rebasing before the final merge git will sort those already existing commits out, leaving only feature related commits.
Now why can't git resolve merges of master into the feature branch like that? According to what you say here, in case I decide to go nuts and cherry pick every single commit in master into my feature branch (those which aren't in feature of course), git would sort those out when finally merging feature into master. What's the difference to merging master into feature? Isn't that basically applying all commits in master which aren't in feature to feature? So that only the commits in feature which aren't in master comprise the difference between feature and master?

Obviously that's not how it works, though, and I assume there is good reason for it, just whining a little bit... ;)


Top
 Profile  
 
PostPosted: Sun Apr 17, 2016 3:15 pm 
Offline
Programmer
User avatar

Joined: Fri Mar 01, 2013 9:52 am
Posts: 1040
Location: Germany
Vezzra wrote:
AFAIK the best way to clean that up in case you find yourself in this situation (having commits you haven't pushed to a branch that got rebased), is to check out a new local copy of the remote branch and cherry pick your commits from the "old" local branch to the new one (Marcel, did I get that right?).


Yes, that's one way (I would suggest as project default). But as usual in Git there multiple ways + 1 other: One would be `git rebase --onto <local copy>` `treeish containing your commits`. It always depends on what you want to achieve exactly.

Quote:
Now why can't git resolve merges of master into the feature branch like that? According to what you say here, in case I decide to go nuts and cherry pick every single commit in master into my feature branch (those which aren't in feature of course), git would sort those out when finally merging feature into master. What's the difference to merging master into feature?


That's a good question actually because I never used the suggested `git checkout feature ; git merge master ; git rebase` workflow.

_________________
Resident code gremlin
Attached patches are released under GPL 2.0 or later.
Git author: Marcel Metz


Top
 Profile  
 
PostPosted: Sun Apr 17, 2016 7:28 pm 
Offline
Creative Contributor
User avatar

Joined: Fri Jun 28, 2013 11:45 pm
Posts: 3291
I've always simply rebased my personal feature branches against Master (and had master set as the upstream generally) when I've needed to, but I'm rarely collaborating with others as much as we'll need to with Fighters, so I don't know how many extra headaches that causes.

The reason this specific branch needed rebasing at one point was because Geoff had added in some extra stuff that was Fighter specific to master after the branch was created by Morlic on his fork but before we decided to have a specific branch. That's meant that, for example, some of the parts with the new exclusion/exception restrictions weren't so restricted when I was testing various ship ideas and it was going to cause a clash. There was another, different, case (oh, yes, an AI bugfix that applied to both branches and was needed in both).

When I've rebased branches with cherry picked content I've had occasional problems and had read (somewhere, not here) that it wasn't a good workflow but I can't remember where or why), I'm very happy to cherry pick stuff in, and also to rebase on occasions if/when needed, but if all fighter related stuff goes into the fighters branch (*cough* Geoff *cough*) until we're all convinced it's ready then we're good.

_________________
Mat Bowles

Any code or patches in anything posted here is released under the CC and GPL licences in use for the FO project.


Top
 Profile  
 
PostPosted: Sun Jun 05, 2016 7:00 pm 
Offline
AI Contributor
User avatar

Joined: Tue Jun 24, 2014 9:55 pm
Posts: 444
I have feeling that I made error in rebasing of Fighters branch.

1) Where I can find what was commited before I rewrite history?
2) Can someone who did not pull yet make reserve copy of Fighters branch?

_________________
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0


Top
 Profile  
 
PostPosted: Sun Jun 05, 2016 7:46 pm 
Offline
Creative Contributor
User avatar

Joined: Fri Jun 28, 2013 11:45 pm
Posts: 3291
Have done so, the most recent commits I have are the ones I made before your three from today, I need to log off now but let me know if you want me to do more/push it, etc and I'll get it done at some point tomrrow.

_________________
Mat Bowles

Any code or patches in anything posted here is released under the CC and GPL licences in use for the FO project.


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 188 posts ]  Go to page Previous  1 ... 8, 9, 10, 11, 12, 13  Next

All times are UTC


Who is online

Users browsing this forum: AhrefsBot and 1 guest


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
Powered by phpBB® Forum Software © phpBB Group