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

Accommodate org.mozilla.rhino 1.8.0 API breakage #2030

Merged
merged 1 commit into from
Jan 14, 2025

Conversation

merks
Copy link
Contributor

@merks merks commented Jan 14, 2025

No description provided.

@hvbtup
Copy link
Contributor

hvbtup commented Jan 14, 2025

@speckyspooky One of the changes of Rhino 1.8.0 in comparison to 1.7.x is that ES6 is now the default, see https://github.com/mozilla/rhino/releases/tag/Rhino1_8_0_Release. Can this break old reports which do not explicitly specify the ES version and assume an older ES version?

BTW the Rhino homepage still says that 1.7.5 is the latest release.

- Update tests expectations as well.
@merks
Copy link
Contributor Author

merks commented Jan 14, 2025

I'm not the expert here. I'm just trying to get builds to work and keep libraries up-to-date.

I didn't get an answer to this question:

mozilla/rhino#1788

I don't see a 1.7.5 release here:

https://repo1.maven.org/maven2/org/mozilla/rhino/

@merks
Copy link
Contributor Author

merks commented Jan 14, 2025

@hvbtup @speckyspooky @wimjongman

I'm not 100% sure I should merge this. Builds will remain broken until this is merged, and I don't think trying to stick to rhino 1.7.15 is a great option. Note that the API changes in 1.8.0 are such that I changed the lower bound to 1.8.0, i.e., the new constructor we are using did not exist in 1.7.15:

image

@merks
Copy link
Contributor Author

merks commented Jan 14, 2025

@wimjongman

In case it's not clear, I've already workaround around the missing OSGi metadata for the 1.8.0 version so we don't strictly require anything from rhino. I was mostly just curious about the intent and about future expectations for future releases.

@hvbtup
Copy link
Contributor

hvbtup commented Jan 14, 2025

I don't see a 1.7.5 release here

Sorry, that was a typo. I meant1.7.15. Seems like the maintainers of Rhino have difficulties with updating their docs for new releases.

@wimjongman
Copy link
Contributor

@merks I'm, not sure what the issue is and what to do. Are you saying your changes are not needed, or are they needed when we upgrade to 1.8

@merks
Copy link
Contributor Author

merks commented Jan 14, 2025

I'm saying that I don't need the rhino folks to do anything in order to produce a bundle for 1.8.0 because that's already done in Orbit.

All the changes in this PR are necessary in order to compile against rhino 1.8.0 and in order for the tests to pass when running using rhino 1.8.0. The only alternative to all these changes is to continue to use rhino 1.7.15 which in my opinion is a bad option. So I think we should just hit the big green Rebase and merge button and move forward.

@wimjongman
Copy link
Contributor

Yes, let's go!

@merks merks merged commit 54e4691 into eclipse-birt:master Jan 14, 2025
3 checks passed
@merks merks deleted the pr-rhino-1.8 branch January 14, 2025 12:15
@speckyspooky
Copy link
Contributor

@hvbtup
Hi Henning, we have no issue with the new standard of ES6 because this is our default since I added the global settings of it.
So I expact no broken reports due to javascript ES6.

@speckyspooky speckyspooky added this to the 4.19 milestone Jan 17, 2025
@speckyspooky speckyspooky added Dependencies Pull requests that update a dependency file Enhancement Small change to improve the current supported functionality labels Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dependencies Pull requests that update a dependency file Enhancement Small change to improve the current supported functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants