[FOSS-GPS] Re: Integrating osm-gps-map into FoxtrotGPS (was: Instance-IDs for osm-gps-map's `image' API)

[Sander van Grieken]

On Tuesday 25 May 2010 15:49:16 Joshua Judson Rosen wrote:
> Sander van Grieken <sander at 3v8.net> writes:
> >
> > It was on my wishlist too BTW, these POI class specific icons, but I
> > don't want to start with new functionality before my branch gets
> > merged to trunk. Can you please consider merging my branch soon?
> 
> At the very least, I'd like to work something out with John about the
> zoom-decoupling before we convert to osm-gps-map; most of the other
> issues that I have with osm-gps-map are things that could be worked
> around if we needed to, and they're all things for which I think John
> is willing to accept patches into osm-gps-map. We need to resolve
> the zoom-decoupling issue, though--I expect that the related menu-items
> have been renderered non-functional in your branch, 

Yes, since there's no original tile painting code anymore, it all went to osm-gps-map and 
of course it doesn't have the zoom-decoupled functionality yet, so I can't hook it up.

> though there are
> a couple of other regressions that prevent me from actually seeing
> (why is it trying to download maps to the root directory?).

It's downloading maps to ~/.cache/osmgpsmap here..

> > Everything is now integrated (so also the friends, POI and photo
> > icons), quite a number of pre-existing bugs are fixed too, and the
> > few things that are not yet possible with the current osm-gps-map
> > (like line-drawing in distance mode) are annotated with TODOs in the
> > code. The code has changed much, so any work based on trunk now will
> > be difficult to merge later.
> 
> You actually have several orthogonal things going on in that branch,
> which makes it difficult to merge: the biggest one is that you've
> integrated osm-gps-map, but you've also done some redesign of the GUI,

No I haven't actually changed the GUI. I have replaced one toolbutton with a 
tooltogglebutton though..

> renamed variables, renamed callbacks, moved functions to different
> files, 

Yes. Since the statements within those callbacks totally changed anyway I went ahead and 
gave them more intuitive names, but AFAIR *only* on the callbacks that were impacted 
anyway.

Maybe that makes the changes a bit harder to follow as an observer, but it's more easily 
understandable by future contributors.

> and made other stylistic changes to unrelated segments of code.

Yes and 99% of it is empty lines. There was lots of that. Maybe Marcus uses a portrait 
flatpanel? :)

Haven't actually tested it, but it should be possible to ignore the whitespace in the 
diffs.

> It looks like there was also an unnecessary merge-conflict created and
> resolved unfavourably in at least main.c (there is a regression in the
> "--gui=" command-line option, if I merge your branch).

Oh? My branch was rebased to trunk right before I sent out the merge request last 
saturday, and it should have been conflict free then. But yeah, the --gui option 
regressed, and is fixed now.

> Can you prepare a branch that has just the changes necessary to integrate
> osm-gps-map, without superfluous changes like the ones listed above

Nope. It means I have to spend a whole day to revert changes that are rejected on an 
esthetics basis and will later probably change anyway. You're right that I should have 
been more conservative in what to change, but being strict about this is mostly only 
useful in projects with many contributors that work on the same files.

Of course I'll prepare my branch by keeping it merge-conflict free, so when/if you decide 
to merge it will be easy (but invasive, yes)

> (including not adding the osm-gps-map dpad & crosshair)?

Fixed.

grtz,
Sander