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

Externalize gradle/maven wrapper command from codestart template #12610

Merged
merged 1 commit into from
Oct 15, 2020

Conversation

glefloch
Copy link
Member

@glefloch glefloch commented Oct 8, 2020

This branch aims to externalize the build tool command used in the generated github action workflow.

@ia3andy I added a field build-cmd which could be used by all command such as:

cmd:
      dev: ${buildtool.cmd.build-cmd} compile quarkus:dev
      package: ${buildtool.cmd.build-cmd} package

But I don't know if this is the goal.

also, I don't how to test if my code is correct, could you give me pointers to validate it ? thanks :)

close #12438

@glefloch glefloch requested a review from ia3andy October 8, 2020 18:52
@ia3andy
Copy link
Contributor

ia3andy commented Oct 9, 2020

@glefloch that's not a bad idea, but it means you have to update all the different commands (and usage), but it would save us from overriding all the different commands where the arguments are the same..

For the GitHub Action issue, the problem is with the CI container which need to include either mvn or gradle.. I guess, instead of having a wrapper data, we could have a new shared-data named container-image or something (that we override in the wrapper).. problem then is with java version..

I let you figure out the best solution :)

@glefloch
Copy link
Member Author

glefloch commented Oct 9, 2020

ah ok, I get you mean for action, in fact, if you don't use the wrapper, you need the right github action. I will update my branch.

I just have another quick question, who does edit this file? Is that generated from https://code.quarkus.io/ ?

@ia3andy
Copy link
Contributor

ia3andy commented Oct 9, 2020

@glefloch for now, it's used by code.quarkus.io (https://github.com/quarkusio/code.quarkus.io/blob/master/src/main/kotlin/io/quarkus/code/service/QuarkusProjectService.kt#L58) when github is picked (and always with the wrappers) so no problem. We don't have code for now that uses github action without the wrapper, but the CreateProject allows it..

@glefloch glefloch marked this pull request as ready for review October 10, 2020 07:04
@glefloch
Copy link
Member Author

@ia3andy, I added some test to assert the condition. Regarding maven, it seems to be available by default on ubuntu-latest nodes. we just have to handle the gradle part.

Copy link
Contributor

@ia3andy ia3andy left a comment

Choose a reason for hiding this comment

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

I added a few simplification ideas 👆

@ia3andy
Copy link
Contributor

ia3andy commented Oct 13, 2020

@glefloch and thanks for doing this :)

@glefloch
Copy link
Member Author

@ia3andy I updated the branch with the changes we discussed.

Copy link
Contributor

@ia3andy ia3andy left a comment

Choose a reason for hiding this comment

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

beside build-ci-args it looks good :)

@ia3andy ia3andy added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 14, 2020
@ia3andy
Copy link
Contributor

ia3andy commented Oct 14, 2020

@glefloch you will need to resolve the conflict introducted by: #12700

Sorry I had to merge it first as it will need to be backported..

@ia3andy
Copy link
Contributor

ia3andy commented Oct 14, 2020

@glefloch we will also need to wait for: #12707

@glefloch
Copy link
Member Author

ok no problem

Copy link
Contributor

@ia3andy ia3andy left a comment

Choose a reason for hiding this comment

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

Well done!

@ia3andy ia3andy merged commit c7713be into quarkusio:master Oct 15, 2020
@gsmet gsmet changed the title Externaliza gradle/maven wrapper command from codestart template Externalize gradle/maven wrapper command from codestart template Oct 27, 2020
@gsmet gsmet added this to the 1.10 - master milestone Oct 27, 2020
@glefloch glefloch deleted the fix/12438 branch August 19, 2021 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/codestarts triage/waiting-for-ci Ready to merge when CI successfully finishes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make sure github action codestarts works without maven/gradle wrapper
3 participants