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

Contribute to exclude property from validation #102

Merged
merged 1 commit into from
Nov 16, 2020
Merged

Contribute to exclude property from validation #102

merged 1 commit into from
Nov 16, 2020

Conversation

angelozerr
Copy link
Contributor

Contribute to exclude property from validation

Fixes #95

Signed-off-by: azerr [email protected]

@angelozerr angelozerr force-pushed the extension-settings branch 2 times, most recently from dd7d9bf to f3132eb Compare November 3, 2020 10:33
@angelozerr angelozerr marked this pull request as ready for review November 3, 2020 10:33
@datho7561
Copy link
Contributor

I will take a look at this today

datho7561
datho7561 requested changes Nov 9, 2020
View reviewed changes
Copy link
Contributor

@datho7561 datho7561 left a comment

Choose a reason for hiding this comment

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

Should the camel.* exclusion apply by default? From my testing, the Camel properties are still validated by default.

microprofile.ls/README.md Outdated Show resolved Hide resolved
...lipse.lsp4mp.ls/src/main/java/org/eclipse/lsp4mp/settings/MicroProfileExtensionSettings.java Outdated Show resolved Hide resolved
...e.lsp4mp.ls/src/test/java/org/eclipse/lsp4mp/settings/MicroProfileExtensionSettingsTest.java Outdated Show resolved Hide resolved
microprofile.ls/README.md Outdated Show resolved Hide resolved
microprofile.ls/README.md Outdated Show resolved Hide resolved
@angelozerr
Copy link
Contributor Author

Should the camel.* exclusion apply by default? From my testing, the Camel properties are still validated by default.

No it's not done by default (it's just for test). @apupier should implement in the camel extension to enable that.

datho7561
datho7561 requested changes Nov 12, 2020
View reviewed changes
Copy link
Contributor

@datho7561 datho7561 left a comment

Choose a reason for hiding this comment

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

I tried to make an extension that would contribute an exclusion for all helidon.* properties, but I can't seem to get it to work. It could just be how I set up the extension though, this is the first time I've done this.

microprofile.ls/README.md Outdated Show resolved Hide resolved
microprofile.ls/README.md Outdated Show resolved Hide resolved
@angelozerr
Copy link
Contributor Author

I tried to make an extension that would contribute an exclusion for all helidon.* properties, but I can't seem to get it to work. It could just be how I set up the extension though, this is the first time I've done this.

It's MP LS extension (like quarkus.ls.ext) and not a MP JDT extension. To test quickly add this settings.json in the quarkus.ls.ext/META-INF/lsp4mp folder and rebuild it.

@angelozerr angelozerr force-pushed the extension-settings branch 2 times, most recently from 9d1dad5 to 6a79315 Compare November 12, 2020 17:56
@datho7561
Copy link
Contributor

Looks good to me!

datho7561
datho7561 requested changes Nov 13, 2020
View reviewed changes
microprofile.ls/README.md
It's possible to ignore this error with contribution (ex : camel toolings can ignore validation for all `camel.*` properties).

To do that in the JAR extension, create the `META-INF/lsp4mp/settings.json` like this:
Copy link
Contributor

Choose a reason for hiding this comment

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

Mention creating a lsp4mp extension, link to the documentation for creating an lsp4mp extension. Then mention that the META-INF/lsp4mp/settings.json should be put into the lsp4mp extension.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@datho7561 there is a new section LSPMP - LS - extensions, please tell me if it's better.

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks good!

@angelozerr angelozerr force-pushed the extension-settings branch 2 times, most recently from 87578eb to 84ed20e Compare November 16, 2020 14:34
@datho7561
Copy link
Contributor

There appears to be a compilation error

@angelozerr angelozerr force-pushed the extension-settings branch 2 times, most recently from 37a5c94 to 4744c63 Compare November 16, 2020 14:56
@angelozerr angelozerr merged commit ec0bea2 into eclipse-lsp4mp:master Nov 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@datho7561 datho7561 datho7561 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Contribute to exclude property from validation
2 participants
@angelozerr @datho7561