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

Updated to openPDF 2.0.2 #662

Merged
merged 7 commits into from
May 27, 2024
Merged

Updated to openPDF 2.0.2 #662

merged 7 commits into from
May 27, 2024

Conversation

bgoorden
Copy link

@bgoorden bgoorden commented Apr 11, 2024

There was already a pullrequest for 2.0.0, but this has some missing changes and failing tests. Since it was not updated for some time I made a new pull request to update to openPDF 2.0.2. (2.0.0 pull request: #636 )

Update to OpenPDF 2.0.2:

#635

OpenPDF require Java 17 or later from version 2.0.0 and later. Before the requirement was java 8.
https://github.com/LibrePDF/OpenPDF/releases/tag/2.0.2 .

I removed the exclusions on bouncycastle. These exclusions are no longer dependencies of openpdf. Also the bouncycastle dependencies that are currently dependencies of openpdf are all optional.

uses: actions/setup-java@v4
with:
java-version: '8'
java-version: '17'
Copy link
Member

Choose a reason for hiding this comment

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

XDocReport is working with Java 8 , why you need Java 17?

Copy link
Author

Choose a reason for hiding this comment

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

Java 17 is required for OpenPDF 2.0.0 or later. The build for this pull request fails with java 8. If we want to upgrade openpdf, I don't think there is another way than to upgrade the minimum java requirement for XDocReport. Of course I'm not sure if losing java 8 support is acceptable for this project.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if losing java 8 support is acceptable for this project.

No sorry, there are so many people who are using XDocReport with old Java version. We need to keep it.

@pascalleclercq do you know if we could build only the module of OpenPDF 2.0.0 with Java 17?

Copy link
Author

Choose a reason for hiding this comment

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

The maven compiler plugin is still set to version 1.7. So if we compile with java 17, XDocReport should still be compatible with java 8, unless you use the openpdf modules of course, right?

Copy link
Author

Choose a reason for hiding this comment

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

I added the changes that would be needed to run the tests with jdk17.

Copy link

Choose a reason for hiding this comment

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

Sorry but the people staying on Java 8 just have to use the old Version of those libraries. For those who keep up to date there should be a release for OpenPDF 2.
You also could make separate Releases for Java 8

Copy link
Member

Choose a reason for hiding this comment

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

Sorry but the people staying on Java 8 just have to use the old Version of those libraries.

You speak about PDF converter, but XDocReport provides another feature like report generation.

@andreasrosdal
Copy link

This is an excellent improvement, which prepares xdocreport for the modern era in Java development. Please merge this pull request.

@angelozerr
Copy link
Member

You also could make separate Releases for Java 8

No time to do that, let's move to Java 17.

@angelozerr
Copy link
Member

@andreasrosdal
Copy link

andreasrosdal commented May 10, 2024

@bgoorden CodeQL must be updated to Java 17 here, can you please update this PR:
https://github.com/opensagres/xdocreport/blob/master/.github/workflows/codeql-analysis.yml

The "Java CI with Maven / build (pull_request)" is succesful.

@andreasrosdal
Copy link

@bgoorden Please update this PR, thank you.

@bgoorden
Copy link
Author

We still want to compile to java 8 code for now right?

@andreasrosdal
Copy link

Java 17, please.

@andreasrosdal
Copy link

There is some build error here, I would appreciate if you would fix this please.

@angelozerr
Copy link
Member

Once ci build wil be fixed we could merge this PR

@bgoorden
Copy link
Author

I'm not getting those build errors on my fork. I updated the maven-bundle-plugin and java version configuration since the error seems to come from there.

@bgoorden bgoorden requested a review from angelozerr May 27, 2024 14:34
@angelozerr
Copy link
Member

LGTM, theCI build is working again, thanksso much @bgoorden !

@angelozerr angelozerr merged commit 483bed8 into opensagres:master May 27, 2024
3 checks passed
@angelozerr angelozerr added this to the 2.0.4 milestone May 27, 2024
@bgoorden bgoorden deleted the Update-to-OpenPDF-2.0.2 branch May 27, 2024 14:43
@angelozerr angelozerr modified the milestones: 2.0.4, 2.0.7 Jun 16, 2024
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.

5 participants