Review of KGeoTag

[Albert Astals Cid]

El dissabte, 28 de novembre de 2020, a les 14:02:28 CET, Tobias Leupold va escriure:
> Dear KDE devs,
> 
> two weeks ago, KGeoTag has been moved to kdereview. I would be very happy if
> this could become an extragear/graphics app, just like KPhotoAlbum (I also
> work on KPA).
> 
> The purpose of KGeoTag is to be a convenient, stand-alone geotagging program
> (but not more than this). Imo, we lack such an app at the latest since
> Digikam's "geolocation" KIPI plugin has been gone (which was usable from
> within KPA back then), without real nice alternatives for the Linux desktop.
> 
> A description of the program can be found in the README at
> https://invent.kde.org/graphics/kgeotag -- it's quite overseeable, but it
> should be feature-complete (at least for a first release).
> 
> I'm only a hobby programmer but I did my best to implement it in a decent way.
> So ... if anybody wants to review it ... :-)

It would be good if you could add a OARS https://hughsie.github.io/oars/ definition on the appdata.xml there's some stuff out there like flathub that enforces having one.

Would it make sense to make File->Add Images also accept folders in addition to individual files?

I'd suggest using KStandardAction::preferences to create your configure action, so you get the default icon all apps get,  same for KStandardAction::quit i guess.

It's not mandatory, something for the future maybe? but i think it'd be nicer if you make your app use kxmlgui, so you don't have to create your own KHelpMenu by hand, and you get one one those configure shortcuts by free, etc.

You have a few signals marked as const, that is a bit weird
	https://invent.kde.org/sdk/clazy/-/blob/master/docs/checks/README-const-signal-or-slot.md

You also have a few methods marked as virtual and override, which is also a bit weird, since all overriden methods are virtual, so you don't need the virtual to be specified.

Cheers,
  Albert

> 
> Cheers, Tobias
> 
> 
> 
>