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

Fix the closure-compiler and closure-library versions #2283

Merged
merged 2 commits into from Jul 2, 2014

Conversation

fredj
Copy link
Member

@fredj fredj commented Jul 1, 2014

@fredj fredj added this to the v3.0.0 milestone Jul 1, 2014
@tschaub
Copy link
Member

tschaub commented Jul 1, 2014

Looks good to me.

@elemoine
Copy link
Member

elemoine commented Jul 1, 2014

@fredj, have you actually verified that ol compiles with these versions of the Closure compiler and library? If so then please merge.

@elemoine
Copy link
Member

elemoine commented Jul 1, 2014

Also, this PR does not seem to be inline with what @tschaub is suggesting in openlayers/closure-util#14 (comment). @tschaub is suggesting to update closure-util to the most recent versions of the Closure Compiler and Library, and update ol3 to the latest closure-util. Any reason for doing otherwise?

To me it makes more sense to do as Tim suggested, because in this way ol3 uses the versions of the Compiler and the Library against which closure-util is tested. And it's only if ol3 doesn't work with the latest versions of the Closure Compiler and Library that we add a closure-util.json file with specific versions to ol3. Does it make sense?

@fredj
Copy link
Member Author

fredj commented Jul 2, 2014

The library version is the same as the version provided with Plovr, the compiler version is the latest one.

I will make a PR to closure-util to update the compiler and library version as Tim suggested but we will still need to set the library version in ol

@fredj
Copy link
Member Author

fredj commented Jul 2, 2014

PR updated, do not merge until closure 0.18.0 is released

@elemoine is that what you had in mind?

The version is the same as the one provided by plovr-81ed862.jar
@elemoine
Copy link
Member

elemoine commented Jul 2, 2014

It looks good to me. I now understand why we need a closure-util.json file in ol3 – the ol3 code is currently not compatible with the latest version of Closure Library.

fredj added a commit that referenced this pull request Jul 2, 2014
Fix the closure-compiler and closure-library versions
@fredj fredj merged commit 242787d into openlayers:master Jul 2, 2014
@fredj fredj deleted the closure-util.json branch July 2, 2014 08:33
@tschaub
Copy link
Member

tschaub commented Jul 2, 2014

It's not clear to me why we wouldn't also peg the Compiler version here.

@probins
Copy link
Contributor

probins commented Jul 7, 2014

the ol3 code is currently not compatible with the latest version of Closure Library.

it would be good to document exactly what is not compatible

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

4 participants