Skip to content
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

Unconditionally add version info to the build header #2446

Merged
merged 1 commit into from
Jul 25, 2014
Merged

Unconditionally add version info to the build header #2446

merged 1 commit into from
Jul 25, 2014

Conversation

tschaub
Copy link
Member

@tschaub tschaub commented Jul 25, 2014

This reworks the changes in #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 reworks the changes in #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.
@marcjansen
Copy link
Member

This looks good to me.

Please merge & thanks.

tschaub added a commit that referenced this pull request Jul 25, 2014
Unconditionally add version info to the build header.
@tschaub tschaub merged commit 4d0fbae into openlayers:master Jul 25, 2014
@tschaub tschaub deleted the version branch July 25, 2014 21:33
@probins
Copy link
Contributor

probins commented Jul 26, 2014

I've been thinking some more about this build header, and there are several issues with having hard-coded text. It's not really appropriate, or not entirely appropriate, in the case where people want to compile their own code in with OL, as the text only applies to the OL bit, not the app code bit. For this, it would be more flexible if the output wrapper were used as before, or perhaps better the new output wrapper file. I would also like the ability to specify the output file in the config compile options rather than in the cmd line; in this case, you would have to use the output wrapper anyway, as the header would never be added. There's also the related question of the license, which should also be added at some point.

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.

None yet

3 participants