[PATCH] Planner notes - revise logic for gasmix output

Dirk Hohndel dirk at hohndel.org
Mon Jun 22 17:53:59 PDT 2015


On Mon, Jun 22, 2015 at 11:09:19PM +0200, Robert C. Helling wrote:
> Hi Rick,
> 
> > On 21 Jun 2015, at 03:13, Dirk Hohndel <dirk at hohndel.org> wrote:
> > 
> > Robert, I consider you the maintainer of the planner. I took Rick's rather
> > straight forward looking patches earlier, but would you take a look at
> > this one before I apply it?
> 
> I am currently looking at the patch and write down things I notice.
> 
> The gas change looks good for our case of a switch at a depth without a stop (even when mandatory stop time is set to 0). But when I turn on displayed transitions I would expect the gas change to be shown on the stop rather than on the upwards transition that leads to it (as the later is only the place to print it if there is no stop for it). I attached a patch (to be applied on top of yours or for yours to be amended with it) to fix that.
> 
> 
> 
> My compiler gives a warning that in that very long if statement there is || next to && and thus some parenthesis might add clarity. I would like to chime in here and also suggest to break that condition over several more lines essentially with one part of the || statement per line and to add comments there so we will not be as confused again when we look at this in a few months.
> 
> Another stylistic thing: The bool gaschange is not a good name anymore and should be renamed to something like gaschange_after and a similar abbreviation gaschange_before could be introduced. Here is a second patch for that:
> 
> 
> 
> But apart from that, this finally seems to do the right thing. Thanks a lot for sorting this out!


Thanks to both of you for working on this.

Because of some of the code cleanup that I did over the weekend these were
kinda hard to apply. I tried to close a few dozen Coverity issues and
ended up changing some of those very same conditional statements since
Coverity convinced me that it was possible for nextdp to be NULL...

Please take a close look at what I just pushed to make sure I didn't end
up breaking things now that you finally got them right.

Thanks

/D



More information about the subsurface mailing list