-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 build configuration to parent #8637
Conversation
Also, remove all uses of profiles outside of parent.
It's a good practice to declare the dependency if a project touches a class from it, without relying on transitive dependencies ("Undeclared but used"). Can you move the javax.annotation part to google-cloud-jar-parent? |
Since we flatten the poms, I guess it will be declared. Declaring and not using a dependency is also a bad practice. So, I'm torn on this one. |
Co-authored-by: Tomo Suzuki <[email protected]>
That's correct, it will be declared anyway. However, skipping dependency declaration (for the flatten plugin's effect) means we can/should remove lots of other dependencies as well. The reasoning would become complex. For example, you had to reason "javax-annotation dependency is already present transitively through api-common". I don't want people spending time on such dependency analysis to decide whether a dependency should be declared. Therefore, I think we should reason about dependency declaration without considering the effect of the flatten plugin. |
<jdk>[9,)</jdk> | ||
</activation> | ||
<dependencies> | ||
<!-- needed for @Generated annotation at compile-time --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice survey.
suztomo@suztomo2:~/google-cloud-java$ grep -ir 'import javax.annotation' . |grep -v Generated
./README.md:import javax.annotation.Nullable;
suztomo@suztomo2:~/google-cloud-java$
Moving this block from jar modules to jar-parent.
Removing this block from module parents. Since
dependencies.sh
is not run at all, it's not clear if these exclusions are still needed.Removing
javax.annotation
declaration for Java 9+, since the dependency is already present transitively throughapi-common
.