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

Use BOM for Micrometer dependency examples in reference docs #5652

Merged
merged 5 commits into from
Nov 18, 2024

Conversation

izeye
Copy link
Contributor

@izeye izeye commented Nov 10, 2024

This PR changes to use Micrometer version for Micrometer dependency examples in reference docs.

See gh-5093

@jonatan-ivanov
Copy link
Member

Should we do this starting from 1.12.x?
I can take care of rebasing/back-porting if we decide so.

@shakuzen
Copy link
Member

Yes, I think we should do it in all the Antora-based docs.

@shakuzen shakuzen added doc-update A documentation update type: task A general task labels Nov 13, 2024
@shakuzen shakuzen added this to the 1.13.8 milestone Nov 13, 2024
@shakuzen
Copy link
Member

I think another option that Jonatan mentioned offline is we could go in the direction of either showing how to configure the BOM in the install snippet, or we could link to another section of the docs that talks about setting up the BOM. Then we could remove the version from individual dependency declarations and mention that it's expected to use the BOM so you don't need to declare explicit versions on dependencies. That is probably better since we do expect the version to be managed for the vast majority of users, and we do not want to encourage people to declare the version at the level of individual dependencies. @izeye would you be willing to update the pull request to that effect if you don't see any issues with that approach?

@izeye
Copy link
Contributor Author

izeye commented Nov 13, 2024

@shakuzen Thanks for the feedback!

Sounds good. I'll try to update this in that direction.

By the way, did you mean to apply this to 1.13.x, not to 1.12.x? 1.12.x also seems to be Antora-based although it seems to be soon to be EOLed for OSS support.

@shakuzen
Copy link
Member

By the way, did you mean to apply this to 1.13.x, not to 1.12.x? 1.12.x also seems to be Antora-based although it seems to be soon to be EOLed for OSS support.

I should have left a comment on that. We should target and apply this to 1.12.x. I made the milestone 1.13 because we don't plan to have another OSS release of 1.12, but we haven't moved the branch to the private repository for enterprise support yet.

@izeye izeye changed the base branch from main to 1.12.x November 13, 2024 16:57
@izeye
Copy link
Contributor Author

izeye commented Nov 13, 2024

@shakuzen Thanks for the explanation!

I switched the base branch to 1.12.x and updated as suggested.

@izeye izeye changed the title Use Micrometer version for Micrometer dependency examples in reference docs Use BOM for Micrometer dependency examples in reference docs Nov 13, 2024
@jonatan-ivanov
Copy link
Member

jonatan-ivanov commented Nov 13, 2024

I added a polish commit (51fe189): moved the templates a level up and found a few other places where it can be used (during forward merge, we should do the same exercise).

@izeye Could you please check if these changes look ok to you?

@izeye
Copy link
Contributor Author

izeye commented Nov 13, 2024

@jonatan-ivanov Looks great. Thank you!

@shakuzen shakuzen modified the milestones: 1.13.8, 1.13.9 Nov 15, 2024
@jonatan-ivanov
Copy link
Member

@izeye I talked to Tommy and we were thinking if it is better to have the BOM instructions in the in install section and everywhere else place a link to it. Tommy's guess was if we duplicate it everywhere, people might not understand and have duplicate BOM declarations.

I made these changes on the PR, what do you think?

Copy link
Contributor Author

@izeye izeye left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good to me.

docs/modules/ROOT/pages/implementations/influx.adoc Outdated Show resolved Hide resolved
docs/modules/ROOT/pages/installing.adoc Outdated Show resolved Hide resolved
Copy link
Member

@shakuzen shakuzen left a comment

Choose a reason for hiding this comment

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

Looks good. I just had one suggestion on wording. Let me know what you think.

docs/modules/ROOT/pages/installing.adoc Outdated Show resolved Hide resolved
Co-authored-by: Tommy Ludwig <8924140+shakuzen@users.noreply.github.com>
@shakuzen shakuzen merged commit 55f90e2 into micrometer-metrics:1.12.x Nov 18, 2024
5 of 6 checks passed
@izeye izeye deleted the gh-5093 branch November 18, 2024 05:00
jonatan-ivanov added a commit that referenced this pull request Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-update A documentation update type: task A general task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants