Skip to content

Conversation

@wendigo
Copy link
Contributor

@wendigo wendigo commented May 15, 2023

We should use either google.inject or javax/jakarta.inject interfaces and annotations.

We will not switch to some DI framework other than Guice, thus having consistent usage of google.inject makes more sense as it's easier to switch to Guice 7.0 in the future which brings support for jakarta.inject namespace.

For reference:

From the code execution perspective, this is a no-op.

Description

Additional context and related issues

Release notes

( ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label May 15, 2023
@wendigo wendigo requested review from electrum and martint May 15, 2023 16:13
@wendigo
Copy link
Contributor Author

wendigo commented May 15, 2023

When #17484 is pulled in, we can add a "restrict import" rule that will prohibit direct usage of javax.inject.*

@github-actions github-actions bot added bigquery BigQuery connector delta-lake Delta Lake connector hive Hive connector hudi Hudi connector iceberg Iceberg connector mongodb MongoDB connector tests:hive labels May 15, 2023
@wendigo wendigo force-pushed the serafin/google-inject branch from 44254d0 to fd56adf Compare May 15, 2023 19:35
@wendigo
Copy link
Contributor Author

wendigo commented May 16, 2023

This PR was prepared semi-automatically using a combination of sed, sift and IntelliJ optimize imports on commit to rearrange import statements and a manual removal of javax.inject dependencies from POMs

@wendigo
Copy link
Contributor Author

wendigo commented May 16, 2023

Recipe:

sift javax.inject.Named -x java -l | while read file; do sed -i '' 's/javax.inject.Named/com.google.inject.name.Named/g' "${file}"; done
sift javax.inject.Qualifier -x java -l | while read file; do sed -i '' 's/javax.inject.Qualifier/com.google.inject.BindingAnnotation/g' "${file}"; done
sift '@Qualifier' -x java -l | while read file; do sed -i '' 's/@Qualifier/@BindingAnnotation/g' "${file}"; done
sift javax.inject.Provider -x java -l | while read file; do sed -i '' 's/javax.inject.Provider/com.google.inject.Provider/g' "${file}"; done
sift javax.inject.Singleton -x java -l | while read file; do sed -i '' 's/javax.inject.Singleton/com.google.inject.Singleton/g' "${file}"; done
sift javax.inject.Inject -x java -l | while read file; do sed -i '' 's/javax.inject.Inject/com.google.inject.Inject/g' "${file}"; done

@wendigo wendigo force-pushed the serafin/google-inject branch 2 times, most recently from 86dcec5 to 3ec3dbb Compare May 22, 2023 15:56
@electrum electrum force-pushed the serafin/google-inject branch 2 times, most recently from 6f15b6f to 1b5d448 Compare June 2, 2023 23:39
wendigo added 3 commits June 2, 2023 19:13
We should use either google.inject or javax/jakarta.inject interfaces
and annotations.

We will not switch to some DI framework other than Guice, thus having
consistent usage of google.inject makes more sense as it's easier to
switch to Guice 7.0 in the future which brings support for
jakarta.inject namespace.
@electrum electrum force-pushed the serafin/google-inject branch from 1b5d448 to 8c169d2 Compare June 3, 2023 02:13
@electrum electrum merged commit df6bec9 into trinodb:master Jun 3, 2023
@github-actions github-actions bot added this to the 419 milestone Jun 3, 2023
@kokosing
Copy link
Member

kokosing commented Jun 3, 2023

Please update modernizer to prevent their return.

@wendigo
Copy link
Contributor Author

wendigo commented Jun 3, 2023

@kokosing not needed. It's banned dependency.

@wendigo wendigo deleted the serafin/google-inject branch June 6, 2023 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bigquery BigQuery connector cla-signed delta-lake Delta Lake connector hive Hive connector hudi Hudi connector iceberg Iceberg connector mongodb MongoDB connector

Development

Successfully merging this pull request may close these issues.

3 participants