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

Added configDir to AugmentPhase #1688

Merged
merged 1 commit into from
Mar 26, 2019
Merged

Added configDir to AugmentPhase #1688

merged 1 commit into from
Mar 26, 2019

Conversation

stalep
Copy link
Member

@stalep stalep commented Mar 26, 2019

Gradle built runners should include application.properties now. Fixes #1645

@stalep stalep added the area/gradle Gradle label Mar 26, 2019
@stalep stalep requested review from gsmet and aloubyansky March 26, 2019 09:07
@Sanne
Copy link
Member

Sanne commented Mar 26, 2019

The CI failures seem related:

Caused by: java.nio.file.NoSuchFileException: D:\a\1\s\devtools\gradle-it\build\resources\main

@Sanne Sanne self-assigned this Mar 26, 2019
aloubyansky
aloubyansky approved these changes Mar 26, 2019
View reviewed changes
Copy link
Member

@aloubyansky aloubyansky left a comment

Choose a reason for hiding this comment

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

Just one remark about the javadoc. Thanks!

core/creator/src/main/java/io/quarkus/creator/phase/augment/AugmentPhase.java Outdated
@@ -131,6 +132,18 @@ public AugmentPhase setWiringClassesDir(Path wiringClassesDir) {
return this;
}

/**
* The directory for generated classes. If none is set by the user,
* wiring-classes directory will be created in the work directory of the creator.
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be re-written for configDir.

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks, fixed it and added a check if configDir is null just to be sure.

@Sanne
Copy link
Member

Sanne commented Mar 26, 2019

I'd normally ask for a test in this case.. but since I have some work in the IT area I'm happy there aren't as they would conflict with my work :)

Let's remember to add some related tests later.

@Sanne Sanne merged commit 5fa9c72 into quarkusio:master Mar 26, 2019
This was referenced Mar 26, 2019
Closed
Open
@gsmet gsmet added this to the 0.13.0 milestone Apr 2, 2019
@vorburger
Copy link
Contributor

Let's remember to add some related tests later.

Adding a non-regression test for whatever this fixed would probably be relatively trivial after #1985 has been merged... you could just add a application.properties with "something testable" (what?) into devtools/gradle-it/src/main/resources and then test whatever that property does in the GradleRunIntegrationTest (introduced in #1985).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aloubyansky aloubyansky aloubyansky approved these changes

@gsmet gsmet Awaiting requested review from gsmet

Assignees

@Sanne Sanne

Labels
area/gradle Gradle
Projects
None yet
Milestone
0.13.0
Development

Successfully merging this pull request may close these issues.
5 participants
@stalep @Sanne @vorburger @aloubyansky @gsmet