-
Notifications
You must be signed in to change notification settings - Fork 3k
Generate all config docs based on new info stored in devtools/core-extensions-json/target/extensions.json #4584
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
Conversation
…tensions-json/target/extensions.json
ObjectMapper mapper = new ObjectMapper(); | ||
|
||
// let's read it (and ignore the fields we don't need) | ||
ExtensionJson extensionJson = mapper.readValue(new File(jsonPath), ExtensionJson.class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll provide you with an API to read the "platform json". You have no choice, atm, but, normally, you shouldn't be making assumptions about its format. There will be multiple consumers of that info, so it'll be better to have a common API to read it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I wait for this API?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could open a PR today/tomorrow. If you need it today, I'll do it today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What you will get is a List<io.quarkus.dependencies.Extension>, which what we've been using to represent extensions described in the JSON so far.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fine, it can wait for tomorrow, we missed the release train :)
docs/src/main/java/io/quarkus/docs/generation/AllConfigGenerator.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @FroMage thanks for this great work again :-). I did a quick look at the PR and I have some two comments/questions.
docs/src/main/java/io/quarkus/docs/generation/AllConfigGenerator.java
Outdated
Show resolved
Hide resolved
docs/src/main/java/io/quarkus/docs/generation/ExtensionJson.java
Outdated
Show resolved
Hide resolved
I believe this is in good enough shape to make the release unless @gsmet and @cescoffier have started. |
@FroMage the release is done. We started early to be able to push to Sonatype before Europe and the US are all pushing artifacts. |
@gsmet OK, np, it'll wait for next week, thanks. |
@aloubyansky what do you know of our CI related to the Maven resolving calls fail there? I'm trying to resolve the extension artifacts from Maven, but apparently they haven't been published on CI yet? |
You should be calling it from a mojo initializing MavenArtifactResolver with the Mojo's repo system, session and repos. |
Well, ATM it's in a |
MAVEN_CMD_LINE_ARGS could help. Bootstrap checks its value to use correct settings and args https://github.com/quarkusio/quarkus/blob/master/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/resolver/maven/MavenRepoInitializer.java#L93 |
Yeah, that fixed it. ATM the failing test in CI is keycloak, we can ignore it. |
…ng to non-existent (here) layouts
@aloubyansky can we get this in and refactor the extension parsing later? |
I was about to add to my Draft PR and make it non-draft. If yours passes though, i think we can get yours in and i'll adjust it myself. |
That'd work for me. |
@gsmet can we merge this? the test failure is keyclock (not related) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't fully reviewed this one but we need it to make progress so let's merge it now.
Thanks a lot! @aloubyansky feel free to refactor this when you merge yours :) Let me know if you need any help! |
This uses the new info generated by @aloubyansky to display the extension names in the "all config" documentation page.
Part of #3803