FreeOrion

Forums for the FreeOrion project
It is currently Tue Dec 12, 2017 4:22 am

All times are UTC




Post new topic Reply to topic  [ 15 posts ] 
Author Message
PostPosted: Fri Aug 11, 2017 1:01 pm 
Offline
Release Manager, Design
User avatar

Joined: Wed Nov 16, 2011 12:56 pm
Posts: 4287
Location: Sol III
This topic is dedicated to the discussion of PRs/bugfix commits which have been proposed for inclusion into the 0.4.7.1 bugfix release.

If you think a PR or a bugfix commit should be included into 0.4.7.1, please create a respective entry in the "Proposed" column of the 0.4.7.1 Bugfix Release project on github, and also post a request explaining why you think that PR/bugfix commit should be included here in this thread. Then the proposed inclusion of that PR/bugfix commit shall be discussed here, and a decision should be reached whether the proposal is accepted or rejected.

An accepted PR/bugfix commit will have it's entry in the 0.4.7.1 Bugfix Release project moved to the "Accepted" column. PRs will also be set to the 0.4.7.1 Bugfix Release milestone and be tagged as "cherry-pick for release". A rejected PR/bugfix commit will have it's entry in the 0.4.7.1 Bugfix Release project moved to the "Rejected" column.

Conditions a PR/bugfix commit must meet to be eligible for proposal:

  • It must not have been proposed before (that is, not be in any columns of the 0.4.7.1 Bugfix Release project).
  • It must be a pure bugfix. No PRs/commits that introduce any kind of new feature, mere tweak, etc.
  • It must be a fix for a bug in the 0.4.7 release, not something that has been introduced in master after the creation of the original 0.4.7 release branch.


Top
 Profile  
 
PostPosted: Fri Aug 11, 2017 1:23 pm 
Offline
Release Manager, Design
User avatar

Joined: Wed Nov 16, 2011 12:56 pm
Posts: 4287
Location: Sol III
Two PRs I want to propose for inclusion (I also mentioned them in the "0.4.7.1 bugfix release?" thread, where a brief discussion already took place):

PR#1603: The issue, while not outright game breaking, is serious/annoying enough and it's fix simple enough to warrant inclusion in a bugfix release IMO. Potentially getting predictions that far off the mark can lead to misguided decisions if you're not aware of the issue.

PR#1599: I think Dilvish summed up the reasons why this should be included pretty well, so I'm just going to quote his statement in the other thread:
Dilvish wrote:
The implementation in 0.4.7 has a number of clear flaws that I don't think anyone has argued against being considered flaws: (a) it implemented a significant design change without any discussion and without even being reflected in the commit message (they just referred to grooming and a dump message) as if it sort of slipped in by accident, (b) it imposes a penalty that is often significantly more severe than is even needed to achieve the design goal cited in its support, and (c) it imposes that new penalty silently, without warning to the folks who would expect things to keep working like they had.

The implementation in master, which I think for the purposes of this discussion is essentially the same as it had been for years without complaint from anyone, including yourself Geoff, is one that at least myself and Mat and Vezzra consider to be a reasonable balancing of multiple design considerations.


Top
 Profile  
 
PostPosted: Fri Aug 11, 2017 6:00 pm 
Offline
Creative Contributor
User avatar

Joined: Fri Jun 28, 2013 11:45 pm
Posts: 3289
If we're doing one, then Move Phototrophic effects evaluation after all other effects by Morlic-fo · Pull Request #1615 · freeorion/freeorion and Prevent unowned monsters from attacking adequately stealthed ships by dbenage-cx · Pull Request #1582 · freeorion/freeorion would both make sense, I was never able to replicate either problem so they were real edge case problems but if you're in that edge case I can see it would be a pain.

Agree on the other two, I should've spotted the proportional production change while testing.

_________________
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 Aug 11, 2017 6:47 pm 
Offline
AI Contributor

Joined: Tue Feb 17, 2015 11:54 am
Posts: 224
Series of commits that fix the AI max population calculation for colonization planning:

https://github.com/freeorion/freeorion/ ... a3ced4cc7e
https://github.com/freeorion/freeorion/ ... ab6c829a57
https://github.com/freeorion/freeorion/ ... 227fa31be6
https://github.com/freeorion/freeorion/ ... acd69f7eef

Was related to e.g. a bug discussed in this thread: viewtopic.php?f=28&t=10630

The maximum note size prevented me from adding a single entry to the proposed column...

_________________
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: Sat Aug 12, 2017 2:56 am 
Offline
AI Lead, Programmer
User avatar

Joined: Sat Sep 22, 2012 6:25 pm
Posts: 4390
Although it has not yet been accepted/merged, and surely would merit some more testing that what I have done myself so far, I think that the newly submitted fix for "false" mandatory fleet stops should at least be considered for inclusion. Although the problem being dealt with there is fairly minor, it is also a fairly obvious vexation which would be nice to have cleared up.

_________________
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 Aug 13, 2017 5:44 pm 
Offline
Release Manager, Design
User avatar

Joined: Wed Nov 16, 2011 12:56 pm
Posts: 4287
Location: Sol III
@Mat: I've added the two PRs you suggested to the 0.4.7.1 bugfix release project.

Morlic wrote:
The maximum note size prevented me from adding a single entry to the proposed column...
No problem.

Dilvish wrote:
Although it has not yet been accepted/merged, and surely would merit some more testing that what I have done myself so far, I think that the newly submitted fix for "false" mandatory fleet stops should at least be considered for inclusion. Although the problem being dealt with there is fairly minor, it is also a fairly obvious vexation which would be nice to have cleared up.
As the fix is very simple, sounds reasonable. However, it needs to go in in time.

@Everyone, please take the time to at least briefly look at the PRs/bugfix commits proposed so far, and voice your objections if you have any. If possible within the next week.


Top
 Profile  
 
PostPosted: Sun Aug 20, 2017 3:47 pm 
Offline
Release Manager, Design
User avatar

Joined: Wed Nov 16, 2011 12:56 pm
Posts: 4287
Location: Sol III
As there have been no objections against the bugfixes proposed for 0.4.7.1 so far, I went ahead and accepted all of them. 0.4.7.1. RC1 has been scheduled for coming Thursday, August 24th, provided no more bugfixes will be suggested (or those that are can be accepted in time). Otherwise RC1 (and the entire schedule for 0.4.7.1 of course) will be postponed accordingly.

So, if anyone wants to propose further bugfixes for inclusion into 0.4.7.1, please do so until Wednesday evening (UTC) at the very latest.


Top
 Profile  
 
PostPosted: Sun Aug 20, 2017 5:21 pm 
Offline
Release Manager, Design
User avatar

Joined: Wed Nov 16, 2011 12:56 pm
Posts: 4287
Location: Sol III
MatGB wrote:
When trying to cherry pick this I found out that this PR fixed a bug introduced after the 0.4.7 release, so it does not apply for the bugfix release.

Consequently has been rejected.


Top
 Profile  
 
PostPosted: Sun Aug 20, 2017 11:35 pm 
Offline
Creative Contributor
User avatar

Joined: Fri Jun 28, 2013 11:45 pm
Posts: 3289
Fair enough, it was one of those edge case "never seen that myself" bugs that I knew was there, didn't know when it came in.

_________________
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: Sat Aug 26, 2017 5:35 pm 
Offline
AI Lead, Programmer
User avatar

Joined: Sat Sep 22, 2012 6:25 pm
Posts: 4390
I am noting dbenage-cx has proposed his starlane fix. Although he only suggested it if something else prompts another RC, this sounds to me like a significant enough fix to merit doing another RC just for it (the issue may be rarely problematic, but it is very problematic on the affected platform).

_________________
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 Aug 27, 2017 11:01 am 
Offline
Release Manager, Design
User avatar

Joined: Wed Nov 16, 2011 12:56 pm
Posts: 4287
Location: Sol III
I'm a bit torn on that. Yes, it's definitely a serious issue on those system where it actually turns up, but, as mentioned, it rarely turns up, and please keep in mind that I can't do a RC2 before Sunday, September 3rd, due to me being away until then. Then roughly another week to give some time to test RC2, which means declaring the release will be postponed at least a week.

So I'd like the others to vote on this proposal: RC2 even if nothing else turns up or not? Is getting in a fix for an issue that, altough serious, only rarely turns up, worth a week's delay?


Top
 Profile  
 
PostPosted: Sun Aug 27, 2017 8:42 pm 
Offline
AI Lead, Programmer
User avatar

Joined: Sat Sep 22, 2012 6:25 pm
Posts: 4390
Vezzra wrote:
Then roughly another week to give some time to test RC2, which means declaring the release will be postponed at least a week.
To me, an extra week's delay does not seem like a big cost for a more thorough bugfix release. Although it's possible that this issue is heavily hardware dependent or may have other aspects limiting the scope of the problem, we don't really know and don't really want to take extra time to figure that out. Fedora 26 is going to be supported for about another year (so would hopefully distribute this bugfix release), and Fedora 27 might easily have this same issue and is due out sometime within about 6 months, perhaps too soon for them to go with 0.4.8.

That said, if dbenage-cx has some reason to think this is just a very flukey Fedora-hardware combo problem, perhaps I am just tilting at a windmill.

_________________
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: Mon Aug 28, 2017 11:38 am 
Offline
Release Manager, Design
User avatar

Joined: Wed Nov 16, 2011 12:56 pm
Posts: 4287
Location: Sol III
Sounds reasonable. Ok, unless anyone objects, I'm going to accept the proposed bugfix for 0.4.7.1 on Sunday and produce RC2.


Top
 Profile  
 
PostPosted: Mon Aug 28, 2017 3:32 pm 
Offline
Programmer

Joined: Mon Feb 29, 2016 8:37 pm
Posts: 204
I agree with Dilvish the starlane fix should be included even if it delays the bugfix release.

Of all the candidates for inclusion in a bugfix relase, I think it has the strongest case. The bug makes the game difficult to play and the fix is of limited scope.


Top
 Profile  
 
PostPosted: Sun Sep 03, 2017 1:15 pm 
Offline
Release Manager, Design
User avatar

Joined: Wed Nov 16, 2011 12:56 pm
Posts: 4287
Location: Sol III
PR#1695 has been accepted and cherry picked.


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 15 posts ] 

All times are UTC


Who is online

Users browsing this forum: No registered users 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