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.

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

Introduce an attribute for the platform groupId #33745

Merged
merged 3 commits into from
Jun 7, 2023

Conversation

gsmet
Copy link
Member

@gsmet gsmet commented May 31, 2023

/cc @inoxx03 here you go for what you requested.

@ebullient I'm a bit concerned by the first commit: your metadata checking tool reported some errors but apparently only because I modified the file. I think we should at least run it on all the files that are around once to fix the failures. And maybe we should also trigger a full run when it is modified.

@inoxx03 the second commit concerns me a bit. It's not the first time that I fix a formatting error that goes unnoticed for a long while. At least from time to time, the doc team should have a look at the output of the Maven doc build as some errors are unfortunately logged at debug level and are not triggering a build failure.

@github-actions
Copy link

github-actions bot commented May 31, 2023

🙈 The PR is closed and the preview is expired.

@ebullient
Copy link
Member

ebullient commented May 31, 2023

/cc @inoxx03 here you go for what you requested.

@ebullient I'm a bit concerned by the first commit: your metadata checking tool reported some errors but apparently only because I modified the file. I think we should at least run it on all the files that are around once to fix the failures. And maybe we should also trigger a full run when it is modified.

I am trying to facilitate incremental improvement by not spamming the universe with all of the errors (especially for errors related to docs you didn't touch). We can trigger analysis of everything (it's a change in parameter when you invoke the test), but I think that's an activity for a different PR.

@inoxx03 the second commit concerns me a bit. It's not the first time that I fix a formatting error that goes unnoticed for a long while. At least from time to time, the doc team should have a look at the output of the Maven doc build as some errors are unfortunately logged at debug level and are not triggering a build failure.

I should/could capture this error level in the docs build and make it a build annotation? That was my mistake originally, and a bunch of humans (including me) missed it the first few times around..

@gsmet
Copy link
Member Author

gsmet commented Jun 6, 2023

@inoxx03 @michelle-purcell I let you check it's all good for you on the doc side and that it does what you wanted?

Copy link
Contributor

@rolfedh rolfedh left a comment

Choose a reason for hiding this comment

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

LGTM

@inoxx03
Copy link

inoxx03 commented Jun 6, 2023

Many thanks for implementing this change @gsmet
I think this is exactly what we need.
I'd like to give our writers a chance test this against the downstream publishing tools, but I am fairly sure that this change will allow us to reuse the automated upgrade guide (and potentially other pieces of content) downstream without the need to change the groupId manually.

Copy link
Contributor

@michelle-purcell michelle-purcell left a comment

Choose a reason for hiding this comment

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

@gsmet - Thanks. LGTM too, providing @rolfedh confirms it's usable downstream too, which it hopefully should be. 👍

@rolfedh
Copy link
Contributor

rolfedh commented Jun 6, 2023

@gsmet - Thanks. LGTM too, providing @rolfedh confirms it's usable downstream too, which it hopefully should be. +1

Yes, it's usable downstream, too. Please go ahead.

@ebullient ebullient merged commit 46dd25a into quarkusio:main Jun 7, 2023
@quarkus-bot quarkus-bot bot added this to the 3.2 - main milestone Jun 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants