-
-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Write library version in build files #2442
Conversation
Great! 👍 Plus one for merging this. Do we have any chance that this could also be added to the library as a property? Maybe via appending sth. to the final output: // compiled output including
// ol.version=null;
})(); // end of wrapping IIFE
ol.version='v3.0.0-gamma.3'; or via a simple search and replace? I am just thinking out loud here. |
@marcjansen I don't think it will be necessary to add this as property to the library. Someone who can read out the property can also look at the first lines of the js file. |
+1 |
looks like this assumes that Git is installed, which may not be the case |
it would be nice to have the closure library/compiler version too #1192 (comment) |
@probins Now it will also work properly when Git is not installed (or when the About the Closure library/compiler version: If you have the ol3 version or commit, don't you implicitly know the Closure version? Because the Closure version is specified in one of the configuration files. |
I still think it is very useful to have the version at hand inside a library property. It's more convenient and allows other libraries on top of ol to react according to a version. AFAIK most libraries provide sth. like this. This discussion shouldn't block this PR from being merged, though. Please merge, @tsauerwein. |
Write library version in build files
don't think so. You could, assuming you have git installed, checkout a new branch for that commit-ish, and look in Thanks for tackling this btw. It's been on my list for a long time. |
function writeOutputfile(compiledSource, callback) { | ||
async.waterfall([ | ||
addFileHeader.bind(null, compiledSource), | ||
fse.outputFile.bind(fse, options.output) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is options
defined here? This doesn't look correct to me.
I see that |
@@ -204,6 +205,39 @@ function main(config, callback) { | |||
|
|||
|
|||
/** | |||
* Adds a file header with the most recent Git tag. | |||
* @param {String} compiledSource The compiled library. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use lowercase for primitive types (e.g. string
).
I agree with this. Let's stick with one version. The others can be looked up. |
@tsauerwein hope my comments above make sense. I've incorporated them into #2446. Thanks for adding this in, I like it. I imagine we may want another way to insert version information (the hosted build tool for example will not be using git clones), but that can be addressed separately (e.g. with a build option). |
@tschaub Thanks for your corrections! I was a bit fast with merging, sorry... |
This reworks the changes in openlayers#2442 so we always include the version info, regardless of whether the module is run as main or not. This also addresses a few lint related issues.
This PR adds the most recent Git tag (
git describe --tags
) to the build files.The
ol.js
now looks like this:And
ol-debug.js
:If there are further commits after the tag, you will also see the last commit. For example, after I made this commit, the version changed to
v3.0.0-gamma.3-1-g076f53c
.Note that you'll have to run
git fetch upstream --tags
to get all remote tags.closes #1792