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

chore: consolidate configuration in google-cloud-pom-parent and google-cloud-jar-parent #8521

Merged
merged 54 commits into from
Oct 12, 2022

Conversation

meltsufin
Copy link
Member

@meltsufin meltsufin commented Oct 5, 2022

  • Attempts to centralize all dependency version management in the root pom.xml and the new google-cloud-jar-parent.
  • Introduces the consolidate_config.sh script to help apply the change across all modules
  • Reduces the scope of renovate bot to just the parent poms
  • There are still many locations where versions are declared outside of the root pom and are not managed by release-please. That will be addressed in the follow-up PRs. For now, this PR adds a CI check to find such violations.

Fixes: #8543
Fixes: #8530
Fixes: #8522
Fixes: #8548

@suztomo
Copy link
Member

suztomo commented Oct 5, 2022

If the root pom.xml has dependencyManagement section, ensure the small BOMs do not inherit the root pom.xml
E.g.,
https://github.com/googleapis/google-cloud-java/blob/main/java-accessapproval/google-cloud-accessapproval-bom/pom.xml#L10

(I remember you have a pull request that fixes it. Where is it now?)

@meltsufin
Copy link
Member Author

@suztomo The script is set_parent_pom.sh, but it looks like it has a bug.

@meltsufin
Copy link
Member Author

@suztomo Do we need a parent at all for the boms?

@suztomo
Copy link
Member

suztomo commented Oct 6, 2022

The parent is for release plugins.

@meltsufin meltsufin changed the title chore: centralize dependency management in root pom.xml chore: consolidate configuration in google-cloud-pom-parent and google-cloud-jar-parent Oct 8, 2022
@meltsufin meltsufin requested a review from suztomo October 11, 2022 04:47
@snippet-bot
Copy link

snippet-bot bot commented Oct 11, 2022

No region tags are edited in this PR.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@meltsufin meltsufin requested review from lqiu96 and ddixit14 October 11, 2022 04:47
pom.xml Outdated Show resolved Hide resolved
Copy link
Member

@suztomo suztomo left a comment

Choose a reason for hiding this comment

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

In this pull request, can you add few paragraphs in README.md (or somewhere) that explain why we introduce google-cloud-pom-parent and google-cloud-jar-parent, and their roles (What kinds of elements should they have)? The paragraphs should cover the things you verbally explained yesterday and the maintainers 1 year later will immediately understand the design.

Copy link
Member

@suztomo suztomo left a comment

Choose a reason for hiding this comment

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

I think google-cloud-gapic-bom, google-cloud-pom-parent, and google-cloud-jar-parent all should have the same value but currently it's maintained by different key such as {x-version-update:google-cloud-jar-parent:current}. Can you set them with 1 key?

pom.xml Outdated Show resolved Hide resolved
@meltsufin
Copy link
Member Author

I think google-cloud-gapic-bom, google-cloud-pom-parent, and google-cloud-jar-parent all should have the same value but currently it's maintained by different key such as {x-version-update:google-cloud-jar-parent:current}. Can you set them with 1 key?

True, but for now I like to keep the convention of the annotation matching the artifactId. I now have scripts that rely on this.

@meltsufin
Copy link
Member Author

In this pull request, can you add few paragraphs in README.md (or somewhere) that explain why we introduce google-cloud-pom-parent and google-cloud-jar-parent, and their roles (What kinds of elements should they have)? The paragraphs should cover the things you verbally explained yesterday and the maintainers 1 year later will immediately understand the design.

Added repository structure documentation: 512f907.

@meltsufin meltsufin added the automerge Merge the pull request once unit tests and other checks pass. label Oct 11, 2022
@meltsufin meltsufin added automerge Merge the pull request once unit tests and other checks pass. and removed automerge Merge the pull request once unit tests and other checks pass. labels Oct 12, 2022
@meltsufin meltsufin merged commit 069b30d into main Oct 12, 2022
@meltsufin meltsufin deleted the update-versions branch October 12, 2022 04:30
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants