-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Introduce QuarkusProject and Quarkus project zipping in codegen #9631
Introduce QuarkusProject and Quarkus project zipping in codegen #9631
Conversation
3452ea3
to
1dc58e9
Compare
Putting in WIP, I am going to introduce the new folder zipping as another commit. |
@aloubyansky there is a bug with the maven plugin due to |
ok, found it, it's nothing, I'll push tomorrow.. |
throw new IllegalStateException("Project writer has not been provided"); | ||
} | ||
final ProjectWriter projectWriter = new FileProjectWriter( | ||
invocation.getQuarkusProject().getProjectFolderPath().toFile()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a note, previously a single invocation could have been passed to multiple handlers (e.g. create followed by add extensions) that would use the same project writer instance. It doesn't mean it has to stay that way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not there yet, but to make it more consistent and simple, my intent is to go toward atomic operations (without having to deal with the writer close/save from outside):
- create (directly with or without extensions)
- add extension(s)
- remove extension(s)
Using the same writer would make it more performant when doing multiple of those, but I guess there is no use case for it.
One good example of the risk of having this close/save handled externally is that (as I did without knowing) having multiple writer and closing the one without the modification after ending up with reseting the modification 😅
WDYT?
One thing to keep in mind is the public API should be backward compatible. Our plugins are looking for the latest platform in a given version range. So, inside that range the API should stay compatible. |
The PR is marked as a WIP, so I am not approving it yet. |
Yeah, I was wondering... It would be good to be allowed to break it in one shot for this refactor? I am not sure if this part of the code is used externally by other than code.quarkus.io? Once it's is clean, we can of course lock it.. |
@aloubyansky Ok I added the project zip compression utils f44f980 (inspired from code.quarkus.io with exec support for wrapper files) And I fixed the BuildFile closing bug by removing the close in |
…Tool coupling This is part of quarkusio#8178 This is a good one :-), things starts to get nicer and cleaner (but not quite there yet ^^)..
f44f980
to
d3b2456
Compare
d3b2456
to
2e7b78b
Compare
@aloubyansky CI is failing because quay seems to be down.. |
this is something we should discuss the timing off. I fully agree it needs to happen - but want to try and make it happen in just one release if at all possible. |
shouldn't we just do this ? it would be other only alternative I can think of is the introduction of a separate file where we externalize the range we check so we can adjust it after release and be able to update it once we know incompatibility happens. |
This is part of #8178
This is a good one :-), things starts to get nicer and cleaner (but not quite there yet ^^)..
Introduce
QuarkusProject
in codegen to reduce ProjectWriter & BuildTool coupling, it will slowly replaceBuildFile
andProjectWriter
.I also uniformised the quarkus-cli (I am not sure if it's used) by giving the project folder instead of the pom.xml, it should now also work with Gradle.
Deleted
ZipProjectWriter
which is not used and will be replaced by a way to zip a Quarkus project folder.