Skip to content
This repository was archived by the owner on Sep 26, 2023. It is now read-only.

Conversation

@lqiu96
Copy link
Member

@lqiu96 lqiu96 commented Nov 22, 2022

No description provided.

@lqiu96 lqiu96 requested a review from blakeli0 November 22, 2022 22:25
@lqiu96 lqiu96 marked this pull request as ready for review November 22, 2022 22:26
@lqiu96 lqiu96 requested review from a team as code owners November 22, 2022 22:26
Copy link
Contributor

@blakeli0 blakeli0 left a comment

Choose a reason for hiding this comment

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

This PR makes me think that maybe we should explicitly creating test jars in the three child poms, instead of trying to create test jars for all 5 and skipping 2. It would also make this section much shorter and clearer in each child pom.

@lqiu96
Copy link
Member Author

lqiu96 commented Nov 23, 2022

This PR makes me think that maybe we should explicitly creating test jars in the three child poms, instead of trying to create test jars for all 5 and skipping 2. It would also make this section much shorter and clearer in each child pom.

@blakeli0 Yep, makes sense. The test-jar config comes from shared-configs at: https://github.com/googleapis/java-shared-config/blob/59a7b6d612aee531342410d0adc3ea161c26fea0/pom.xml#L340 so I would still need to explicitly skip for the parent and the BOM.

Do you think it would be cleaner to just add in skip for the parent and the BOM and move

                <include>com/google/api/gax/core/**</include> -> `Move to gax`
                <include>com/google/api/gax/rpc/testing/**</include> -> `Move to gax`
                <include>com/google/api/gax/rpc/mtls/**</include> -> `Move to gax`
                <include>com/google/api/gax/grpc/testing/**</include> -> `Move to gax-grpc`
                <include>com/google/api/gax/httpjson/testing/**</include> -> `Move to gax-httpjson`

to their respective modules?

I added in <skip.generate.testlib> property for clarity, but it might have actually added in more clutter...

@blakeli0
Copy link
Contributor

Do you think it would be cleaner to just add in skip for the parent and the BOM and move

                <include>com/google/api/gax/core/**</include> -> `Move to gax`
                <include>com/google/api/gax/rpc/testing/**</include> -> `Move to gax`
                <include>com/google/api/gax/rpc/mtls/**</include> -> `Move to gax`
                <include>com/google/api/gax/grpc/testing/**</include> -> `Move to gax-grpc`
                <include>com/google/api/gax/httpjson/testing/**</include> -> `Move to gax-httpjson`

to their respective modules?

Yes, that is much cleaner. We also don't need a <skip.generate.testlib> property. Currently we have this section declared in the parent pom with a skip = True, which could be very confusion to other people.

@lqiu96
Copy link
Member Author

lqiu96 commented Nov 28, 2022

@blakeli0 PTAL

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@lqiu96 lqiu96 merged commit de0ca3b into main Nov 28, 2022
@lqiu96 lqiu96 deleted the main-module_skip_jar branch November 28, 2022 18:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants