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.

Sign up for GitHub

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

Jump to bottom

Guice replacement #994

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Guice replacement #994

wants to merge 4 commits into from

Conversation

fbarbero-ecervo
Copy link

No description provided.

@reinhapa
Copy link
Member

reinhapa commented Nov 19, 2024 •
edited
Loading

@fbarbero-ecervo would you mind using the jakarta.inject annotations? Also you may need to change the github action to change the build over to JDK 21..,

@fbarbero-ecervo
Copy link
Author

@reinhapa Yes, I triy with Guice and jakarta.inject. It seems working for now. But I am facing some issues with the "dist" module because your build has been done on Windows, right ?
And yes, I need to update the Github Action as well. I will end my development with this part 😉

@fbarbero-ecervo fbarbero-ecervo changed the title Fabien/cpg5 6613 guice Guice replacement Nov 20, 2024
@fbarbero-ecervo fbarbero-ecervo force-pushed the fabien/CPG5-6613-guice branch 4 times, most recently from a8eb789 to 05f67bb Compare November 21, 2024 12:54
@reinhapa
Copy link
Member

@reinhapa Yes, I triy with Guice and jakarta.inject. It seems working for now. But I am facing some issues with the "dist" module because your build has been done on Windows, right ? And yes, I need to update the Github Action as well. I will end my development with this part 😉

The build is running on Linux only, but I would like to extend that to Windows and Mac even-though the actual usage is on all those platforms...

What was the reason to switch back to Guice 5.1 ?

@fbarbero-ecervo
Copy link
Author

I switch to Guice 5.1 for test only. I will go back to v7.0 soon !

In fact, I try to use the "ant" module in a Gradle plugin, but I found an issue with the "classpath" which does not include Guice. I'm looking for a solution to this problem.

FYI, the compilation on this branch works, but some tests are still failing. I am not sure to have time to fix them 😞

@fbarbero-ecervo fbarbero-ecervo force-pushed the fabien/CPG5-6613-guice branch 5 times, most recently from 245f49a to 8f82bdd Compare November 28, 2024 09:39
fbarbero-ecervo
fbarbero-ecervo commented Nov 28, 2024
View reviewed changes
pom.xml
@@ -165,6 +169,11 @@
<artifactId>tika-parsers</artifactId>
<version>1.28.5</version>
</dependency>
<dependency>
<groupId>org.ow2.asm</groupId>
<artifactId>asm</artifactId>
Copy link
Author

Choose a reason for hiding this comment

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

why did I add this ?!

izpack-util/src/main/java/com/izforge/izpack/util/IoHelper.java
IOUtils.copy(zin, out);
}
out.closeEntry();
} catch (ZipException ex) {
Copy link
Author

Choose a reason for hiding this comment

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

did not understood how it can work before ...

izpack-test/src/test/java/com/izforge/izpack/compiler/container/TestCompilationContainer.java
jos.putNextEntry(new ZipEntry("test"));
jos.closeEntry();
jos.finish();
jos.close();
Copy link
Author

Choose a reason for hiding this comment

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

use an utility method

@fbarbero-ecervo fbarbero-ecervo force-pushed the fabien/CPG5-6613-guice branch 2 times, most recently from c221d0d to bf58d1c Compare December 10, 2024 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fbarbero-ecervo @reinhapa