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

Improve documentation of configuration properties with type Map<String, NotAConfigGroup> #40335

Merged
merged 3 commits into from
May 14, 2024

Conversation

yrodiere
Copy link
Member

@yrodiere yrodiere commented Apr 29, 2024

Fixes #40332

Before the patch:

image

After the patch (without CSS, since I tested locally...):

image

Unfortunately, this PR needs to set the name of the map keys, which will break direct links to the relevant configuration properties in the docs. That's because anchors are generated from the displayed key, including the "some-name" parts... which IMO is a dubious choice. We should probably change that to use anchors generated from some display-independent reference to the key. But that will result in breaking even more links, and anyway won't solve our problem here (since existing links will still be using the old anchor that includes the old "some-name"), so I won't do it in this PR.

Copy link

quarkus-bot bot commented Apr 29, 2024

/cc @brunobat (micrometer), @ebullient (micrometer), @gsmet (elasticsearch), @loicmathieu (elasticsearch)

@yrodiere yrodiere requested a review from gsmet April 29, 2024 09:35
@yrodiere yrodiere force-pushed the i40332-configMapDocs branch from 320698c to ab91604 Compare April 29, 2024 09:36
@quarkus-bot quarkus-bot bot added area/security area/spring Issues relating to the Spring integration labels Apr 29, 2024
@yrodiere yrodiere marked this pull request as ready for review April 30, 2024 12:56
@yrodiere yrodiere added the triage/needs-review Issue that needs a review - remove label if all is clear label Apr 30, 2024
Copy link

quarkus-bot bot commented Apr 30, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 32ee0a9.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@yrodiere
Copy link
Member Author

yrodiere commented May 2, 2024

Damn, Guillaume went on PTO before he had time to review this. I thought he of all people would be interested.

@geoand maybe? :)

@geoand
Copy link
Contributor

geoand commented May 2, 2024

It LGTM, but I not done any work around this area not have I followed discussions so I am hesitant to approve

@yrodiere
Copy link
Member Author

yrodiere commented May 2, 2024

Thanks. Alright, let's wait for @gsmet .
FWIW there's no conversation beside #40332 .

@yrodiere
Copy link
Member Author

Bump @gsmet :)

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

Looks interesting, let's merge, thanks!

@gsmet gsmet merged commit 849d08f into quarkusio:main May 14, 2024
65 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.11 - main milestone May 14, 2024
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label May 14, 2024
@yrodiere yrodiere deleted the i40332-configMapDocs branch June 21, 2024 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment