D8158: KDevelop: decorate patch version string in development builds

[Kevin Funk]

kfunk added inline comments.

INLINE COMMENTS

> rjvbb wrote in CMakeLists.txt:133
> We're not overwriting anything, we're adding an additional full version string. That's not unheard of, this is exactly what's done in Apple's Info.plist files. A "numeric" version for use in code, and a human-readable string that is to be presented to users.
> 
> I can achieve the same thing the way Krita does things, but that's more complicated (cf. the script I used in the initial release). Sven pointed out that we cannot change VERSION_PATCH because that variable is used by dependent code.
> 
> Whatever approach I will use I find it crucial that the output from `git describe` is used. Git hashes are random, so using only them will not allow checking at a glance which of two versions is older or newer (and that's part of what I want to achieve with this patch).

> We're not overwriting anything, we're adding an additional full version string. That's not unheard of, this is exactly what's done in Apple's Info.plist files. A "numeric" version for use in code, and a human-readable string that is to be presented to users.

You're basically overwriting/invalidating the `major.minor.patch` version which was previously shown in the About dialog as set by CMake.

> I can achieve the same thing the way Krita does things, but that's more complicated (cf. the script I used in the initial release). Sven pointed out that we cannot change VERSION_PATCH because that variable is used by dependent code.

It's not more complicated. Please forget about Krita again, it probably only confuses you.

And where did I talk about changing VERSION_PATCH?

`set(KDevelop_FULLVERSION_STRING "${KDEVELOP_VERSION_STRING} (git ${SHA1}")` doesn't "change" VERSION_PATCH.

> Whatever approach I will use I find it crucial that the output from git describe is used. Git hashes are random, so using only them will not allow checking at a glance which of two versions is older or newer (and that's part of what I want to achieve with this patch).

Okay, then please use:
`set(KDevelop_FULLVERSION_STRING "${KDEVELOP_VERSION_STRING} (git ${GIT_FULL_VERSION}")`

That'll show e.g. '5.1.2' twice, but that doesn't harm.

> rjvbb wrote in GetGitRevisionDescription.cmake:1
> Sorry, I am working against the 5.2 branch (it's not a huge change that introduces wildly different behaviour after all).

Hm?

I was asking you to update the copy of those GetGit*.cmake files. What does that have to do with KDevelop's 5.2 branch?

> rjvbb wrote in kdevelop-fullversion.h.cmake:1
> Shouldn't config-kdevelop.h be made consistent too then?

Not as part of this patch, no. Unrelated.

REPOSITORY
  R32 KDevelop

REVISION DETAIL
  https://phabricator.kde.org/D8158

To: rjvbb, #kdevelop, brauch, kfunk
Cc: kossebau, flherne, mwolff, brauch, kdevelop-devel, geetamc, Pilzschaf, akshaydeo, surgenight, arrowdodger
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20171008/9a8d204e/attachment.html>