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

Register user configuration sources #1873

Merged
merged 6 commits into from
Apr 8, 2019
Merged

Register user configuration sources #1873

merged 6 commits into from
Apr 8, 2019

Conversation

dmlloyd
Copy link
Member

@dmlloyd dmlloyd commented Apr 4, 2019

Fixes #1871

@geoand
Copy link
Contributor

geoand commented Apr 4, 2019 •
edited
Loading

Should this be documented perhaps in the configuration guide? Asking because the email from the user asking for this feature mentioned that it wasn't clear from our docs how it could be accomplished.

@dmlloyd
Copy link
Member Author

dmlloyd commented Apr 5, 2019

Sure, I can add a blurb.

@dmlloyd
Copy link
Member Author

dmlloyd commented Apr 5, 2019

Pushed. @geoand WDYT?

@geoand
Copy link
Contributor

geoand commented Apr 5, 2019

@dmlloyd LGTM!

@cescoffier
Copy link
Member

I know it's annoying, but what about a test?

@dmlloyd
Copy link
Member Author

dmlloyd commented Apr 5, 2019

Added tests, found & squashed some other bugs as a result.

@dmlloyd
Copy link
Member Author

dmlloyd commented Apr 5, 2019

I'll have to have someone with a Windows system give me a hand with troubleshooting this, next week.

@starksm64 starksm64 self-assigned this Apr 6, 2019
@starksm64
Copy link
Contributor

I'll take a look at the failure on windows failure when I get back home.

@dmlloyd
Copy link
Member Author

dmlloyd commented Apr 8, 2019

Thanks Scott.

@dmlloyd
Copy link
Member Author

dmlloyd commented Apr 8, 2019

I wonder if maybe one of the relativePath entries is using \ separators instead of /.

@dmlloyd
Copy link
Member Author

dmlloyd commented Apr 8, 2019

Trying one more thing. Since Path instances can exist on any file system with any file separator, just use toUri().getPath() instead.

@dmlloyd
Copy link
Member Author

dmlloyd commented Apr 8, 2019

Never mind, that was wrong: it seems that toUri() always makes the path absolute, unfortunately.

@dmlloyd
Do not use Path.toString() s
7e319b4 
Different filesystems use different separators; standardize on URI paths instead.
@dmlloyd
Copy link
Member Author

dmlloyd commented Apr 8, 2019

Hey looks like that fixed the Windows build! Cool.

@dmlloyd dmlloyd merged commit d0c3873 into quarkusio:master Apr 8, 2019
@dmlloyd dmlloyd deleted the user-sources branch April 8, 2019 17:55
@dmlloyd dmlloyd added this to the 0.14.0 milestone Apr 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kenfinnigan kenfinnigan kenfinnigan approved these changes

Assignees

@starksm64 starksm64

Labels
release/noteworthy-feature
Projects
None yet
Milestone
0.13.1
Development

Successfully merging this pull request may close these issues.
5 participants
@dmlloyd @geoand @cescoffier @starksm64 @kenfinnigan