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

Support microprofile-config.properties on JDT LS side #199

Merged
merged 1 commit into from
Jan 27, 2020
Merged

Support microprofile-config.properties on JDT LS side #199

merged 1 commit into from
Jan 27, 2020
+430 −158

Conversation

angelozerr
Copy link
Contributor

Support microprofile-config.properties on JDT LS side

See redhat-developer/vscode-quarkus#181

Signed-off-by: azerr azerr@redhat.com

@angelozerr
Copy link
Contributor Author

angelozerr commented Jan 24, 2020 •
edited
Loading

This PR gives the capability to load properties from META-INF/microprofile-config.properties at first and application.properties file after.

To test it, you can play with the quarkus.http.port property and codelens URL.

  • you set this property to 8081 in src/main/resources/META-INF/microprofile-config.properties you refresh the Java editor and you should see CodeLens url with 8081.

  • you set this property to 8082 in src/main/resources/application.properties you refresh the Java editor and you should see CodeLens url with 8082.

There is a JUnit test with this usecase.

Please note this PR will help to manage application.yaml by implementing a new IConfigSource (like YAMLConfigSource for issues #195 and #196

@angelozerr angelozerr force-pushed the http-server-mp branch 9 times, most recently from be1b189 to f3109ec Compare January 24, 2020 17:53
@angelozerr
Copy link
Contributor Author

@fbricon test are fixed now.

fbricon
fbricon requested changes Jan 27, 2020
View reviewed changes
...rofile.jdt.core/src/main/java/com/redhat/microprofile/jdt/core/PropertiesManagerForJava.java Outdated Show resolved Hide resolved
...re/src/main/java/com/redhat/microprofile/jdt/internal/core/project/AbstractConfigSource.java Outdated Show resolved Hide resolved
.../src/main/java/com/redhat/microprofile/jdt/internal/core/project/JDTMicroProfileProject.java
public class JDTMicroProfileProject {

public static final String MICROPROFILE_CONFIG_PROPERTIES_FILE = "META-INF/microprofile-config.properties";
public static final String APPLICATION_PROPERTIES_FILE = "application.properties";
Copy link
Collaborator

Choose a reason for hiding this comment

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

those will need to be decoupled eventually

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree we should manage that with new extension point (like we have done for properties provider), but I think we should create a new issue for that which is not trivial.

.../src/main/java/com/redhat/microprofile/jdt/internal/core/project/JDTMicroProfileProject.java

private final List<IConfigSource> configSources;

private static final String QUARKUS_HTTP_PORT = "quarkus.http.port";
Copy link
Collaborator

Choose a reason for hiding this comment

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

will need to be decoupled eventually, as it's a quarkus thing, not an MP thing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree we should manage that with new extension point (like we have done for properties provider), but I think we should create a new issue for that which is not trivial.

@angelozerr angelozerr force-pushed the http-server-mp branch 2 times, most recently from 4acedec to f9e5cd9 Compare January 27, 2020 10:32
@fbricon fbricon merged commit b612beb into redhat-developer:master Jan 27, 2020
@fbricon fbricon mentioned this pull request Jan 28, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fbricon fbricon fbricon requested changes

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
@angelozerr @fbricon