Skip to content

Allow use of BUILD env variable for version BUILD#958

Merged
keneanung merged 7 commits intoMudlet:developmentfrom
keneanung:UseBuildEnvironmentVariable
Apr 27, 2017
Merged

Allow use of BUILD env variable for version BUILD#958
keneanung merged 7 commits intoMudlet:developmentfrom
keneanung:UseBuildEnvironmentVariable

Conversation

@keneanung
Copy link
Copy Markdown
Member

This allows us to set the BUILD part of the version string to anything we like
WITHOUT modifying the actual "Makefiles".

This allows us to set the BUILD part of the version string to anything we like
WITHOUT modifying the actual "Makefiles".
src/src.pro Outdated
# if you are distributing modified code, it would be useful if you
# put something distinguishing into the BUILD environment variable to make
# identification of the used version simple
# the qmake BUILD variable is NO built-in
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe NOT built-in?

Copy link
Copy Markdown
Member

@vadi2 vadi2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you make it so if BUILD is defined, it uses that, otherwise it uses the BUILD value in the makefile? Sometimes you don't want to set it as an environment variable every single time and just edit the makefile once. I think I've seen this in other projects as well.

@keneanung
Copy link
Copy Markdown
Member Author

That's the case already: If the BUILD environment variable is defined and not empty, it gets used. Otherwise, the content of the file is used (-dev by default).

Copy link
Copy Markdown
Member

@vadi2 vadi2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Would you mind applying the same solution for #809 as well?

@keneanung
Copy link
Copy Markdown
Member Author

Will do in a follow-up PR.

@SlySven
Copy link
Copy Markdown
Member

SlySven commented Apr 25, 2017

OK, I think I am still getting my head around this - but is it clear that it works correctly for the "release" builds where the BUILD variable should (end up) being empty?

CMakeLists.txt Outdated
SET(APP_VERSION 3.0.1)
SET(APP_BUILD "-dev")
IF(DEFINED ENV{BUILD} AND NOT $ENV{BUILD} STREQUAL "")
SET(APP_BUILD $ENV{BUILD})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably should be something like MUDLET_BUILD_VERSION or MUDLET_VERSION_BUILD the word BUILD is too common for an env variable.

@ahmedcharles
Copy link
Copy Markdown
Contributor

@SlySven Releases have to be made available in source form and part of that requirement means that you have to actually change these to not have -dev anymore.

@ahmedcharles
Copy link
Copy Markdown
Contributor

Resolved the merge conflicts.

The name `BUILD` is too common to use as a build environment variable.
`MUDLET_VERSION_BUILD` uses a namespace (MUDLET) and is far more specific.
@keneanung
Copy link
Copy Markdown
Member Author

@ahmedcharles have another look?

@SlySven
Copy link
Copy Markdown
Member

SlySven commented Apr 27, 2017

@ ahmedcharles wrote:

@ SlySven Releases have to be made available in source form and part of that requirement means that you have to actually change these to not have -dev anymore.

I think I have got it - I just was initially unsure that when it comes around for Vadim(?) to produce 3.1.0 {in a few days 😉} what needs to be changed so the end result is identified as the "Official" 3.1.0 version is clear - and to me it looked a little less clear that it was - but if the relevant persons can work it out then that is fine. 😁

@keneanung keneanung merged commit 1f31341 into Mudlet:development Apr 27, 2017
@keneanung keneanung deleted the UseBuildEnvironmentVariable branch April 27, 2017 14:05
@vadi2
Copy link
Copy Markdown
Member

vadi2 commented Apr 27, 2017

Sorry I forgot to mention, could you also update the comments for the Makefiles to say that they can be supplied as environment variables? Not everyone will be able to read qmake/cmake.

@keneanung
Copy link
Copy Markdown
Member Author

The comments say

developers may like to set the MUDLET_VERSION_BUILD environment variable to their user and branch names to make it easier to tell different builds apart!

and

if you are distributing modified code, it would be useful if you put something distinguishing into the MUDLET_VERSION_BUILD environment variable to make identification of the used version simple

@vadi2
Copy link
Copy Markdown
Member

vadi2 commented Apr 27, 2017

They do. Time for a power nap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants