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

Migrated and upgraded some libraries to jakarta namespace #285

Merged
merged 18 commits into from
Oct 1, 2024

Conversation

riperat
Copy link
Contributor

@riperat riperat commented Jul 15, 2022

The test HealthCheckTest.shouldRespondToHealthCheckOnNetModule() fails and needs to be fixed.

@uhurusurfa
Copy link
Contributor

@riperat - thank you for submitting a PR.
Can you provide some insight as to why yopu feel OpenAS2 would benfit from using the Jakarta package of the javax.mail API?
It would help to provide technical reasons why you feel this is a good move for the project.

@riperat
Copy link
Contributor Author

riperat commented Jul 18, 2022

@uhurusurfa As you know, Oracle owns the name "Java" trademark, and Java EE has been rebranded to Jakarta EE, necessitating a package relocation. So, since javax packages are deprecated by their Jakarta counterparts, migrating to the Jakarta namespace is a must to use current versions of Mail dependency.

@uhurusurfa
Copy link
Contributor

@riperat - thanks for the insight - I was not aware of the underlying issue of name-spacing.

@uhurusurfa
Copy link
Contributor

@riperat - the CI/CD pipeline is failing. This particular failure is related to Java cacerts that allow it to verify the downloaded packages for some maven zips and the only fix I have figured out is to upload a custm cacerts file that is the very latest available. Please overwrite the cacerts file in the root folder with a newer one and see if it fixes the failing tests.

@uhurusurfa
Copy link
Contributor

Change the file .github/workflow/action.yml.

For the Maven build run command, change it to this:
- name: Build with Maven
run: |
./mvnw clean -e --debug --file pom.xml --log-file maven.log
./mvnw test -e --debug --file pom.xml --log-file maven.log

Not sure why it is now needed but running the "clean" command fixes it on my test branch.

@riperat
Copy link
Contributor Author

riperat commented Oct 4, 2022

@uhurusurfa Is this change going to be introduced in the next release? I'm asking because I see that the CI is failing again after we fixed It.

@uhurusurfa
Copy link
Contributor

The change is incomplete and that is why there are failures.
I will rasie a PR against your branch to fix it.

@riperat
Copy link
Contributor Author

riperat commented Jan 5, 2023

Hi, is there something I can do to make it ready for the next release, I'm asking because I need the change for a project I'm working on.

Copy link
Contributor

@uhurusurfa uhurusurfa left a comment

Choose a reason for hiding this comment

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

Thankls for the PR 🚀

Comment on lines 36 to 37
./mvnw clean -e --debug --file pom.xml --log-file maven.log
./mvnw test -e --debug --file pom.xml --log-file maven.log
Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately the formatting in my PR ocmment got screwed up. You need to indent the 2 lines to execute maven to inline with the | on the "run" line.
See here for example:
https://github.com/OpenAS2/OpenAs2App/pull/286/files#diff-2702f6f7d10a1bffbc8596d1a9925b1234bc4c7a6ff4c21ca6b34c13aa75780e

Copy link
Contributor

Choose a reason for hiding this comment

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

@riperat NOTE: Because the actions.yml file is invalid it has not run the unit tests so although it looks like it has passed, it has needs a fix per above.

@uhurusurfa uhurusurfa merged commit 2570704 into OpenAS2:master Oct 1, 2024
11 checks passed
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.

2 participants