On Wed, Apr 20, 2016 at 01:14:32PM -0700, Linus Torvalds wrote:
> On Apr 20, 2016 12:48 PM, "Dirk Hohndel" <[email protected]> wrote:
> >
> > Reading the patch the code makes sense and seems an improvement. I'm not
> > in love with "int entry" since we have pi->entry (which is a pointer) to
> > confuse it with, but heck, I'm sure you had a good reason for picking that
> > name :-)
> 
> I'd normally call it "index" or something, but it's entry exactly because
> we already had "index" for the time slot index. Gaah.

Which is why I tend to reach for longer variable names. Or single letters
:-)

> > What puzzles me is why we even create 3, 6, and 9 minute intervals since
> > it seems we never use anything but the 9 minute intervals. What am I
> > missing?
> 
> I have this memory of us playing with showing the momentary average depth
> and min/max bands in the profile. The same way we were playing around with
> smoothing etc.

Yes, we had the purple hose... I need to try to build Subsurface 1.0 again
:-)

> So I think it's historical. I don't think we use the average at all, and we
> only use the 9-minute min/max (which is a fairly odd number too).

We started with 3min and that was way too busy. And then 6 and 9 looked
reasonable, I guess. I'm not sure this is worth tuning significantly, but
then again, it maybe fun to look at the results of using different
thresholds.

> If you do decide to just remove the three different time windows and go for
> just one, you could rename "entry" to "index" at the same time.

We're not using it. Then why spend the time and energy to calculate it.
Especially when thinking that this is running on phones...

> And if we decide that the momentary average depth is never going to be
> interesting, that code could be removed too.

I think that's reasonable.

> I'm driving around, so I'm not going to generate that patch. But it should
> be pretty obvious..

I think I'll take what you have and apply it. And then follow up with a
second patch to rip out the 3 and 6 minutes as well as the average. This
way we can easily undo the "ripping out" without losing the important
fixes that are in your patch.

Thanks

/D
_______________________________________________
subsurface mailing list
[email protected]
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

Reply via email to