Skip to content
This repository has been archived by the owner on Jan 19, 2022. It is now read-only.

Added proxyBeanMethods = false to configuration classes for better GraalVM support #2525

Merged
merged 5 commits into from
Sep 20, 2020

Conversation

gabac
Copy link
Contributor

@gabac gabac commented Sep 16, 2020

Fixed #2524 and fixes #2500

I didn't change samples and test configurations.

Copy link
Contributor

@elefeint elefeint left a comment

Choose a reason for hiding this comment

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

You'll have to make GcpCloudSqlAutoConfiguration and GcpStorageAutoConfiguration non-abstract for the tests to pass.
We've made them abstract to appease checkstyle, which no longer looks like a good tradeoff.

@gabac
Copy link
Contributor Author

gabac commented Sep 17, 2020

You'll have to make GcpCloudSqlAutoConfiguration and GcpStorageAutoConfiguration non-abstract for the tests to pass.
We've made them abstract to appease checkstyle, which no longer looks like a good tradeoff.

Thanks for the hint 👍

According to the https://github.com/spring-cloud/spring-cloud-gcp/blob/master/CONTRIBUTING.adoc I'm supposed to run

$ bash util/update-license-year.sh

this is failing for me with:

Updating year to 2020 on files different from HEAD
xargs: illegal option -- -

@codecov
Copy link

codecov bot commented Sep 17, 2020

Codecov Report

Merging #2525 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2525   +/-   ##
=========================================
  Coverage     74.27%   74.27%           
  Complexity     2162     2162           
=========================================
  Files           267      267           
  Lines          7760     7760           
  Branches        803      803           
=========================================
  Hits           5764     5764           
  Misses         1626     1626           
  Partials        370      370           
Flag Coverage Δ Complexity Δ
#unittests 74.27% <100.00%> (ø) 2162.00 <1.00> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ Complexity Δ
...nfigure/bigquery/GcpBigQueryAutoConfiguration.java 81.25% <ø> (ø) 3.00 <0.00> (ø)
...figure/config/GcpConfigBootstrapConfiguration.java 0.00% <ø> (ø) 0.00 <0.00> (ø)
...utoconfigure/core/GcpContextAutoConfiguration.java 83.33% <ø> (ø) 7.00 <0.00> (ø)
...astore/DatastoreRepositoriesAutoConfiguration.java 100.00% <ø> (ø) 1.00 <0.00> (ø)
.../DatastoreTransactionManagerAutoConfiguration.java 80.00% <ø> (ø) 1.00 <0.00> (ø)
...igure/datastore/GcpDatastoreAutoConfiguration.java 85.71% <ø> (ø) 17.00 <0.00> (ø)
...tastore/GcpDatastoreEmulatorAutoConfiguration.java 72.41% <ø> (ø) 6.00 <0.00> (ø)
...lth/DatastoreHealthIndicatorAutoConfiguration.java 100.00% <ø> (ø) 2.00 <0.00> (ø)
...estore/FirestoreRepositoriesAutoConfiguration.java 100.00% <ø> (ø) 1.00 <0.00> (ø)
...igure/firestore/GcpFirestoreAutoConfiguration.java 91.17% <ø> (ø) 4.00 <0.00> (ø)
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fda989f...00a3fd1. Read the comment docs.

@gabac gabac requested a review from elefeint September 17, 2020 16:38
@dzou
Copy link
Contributor

dzou commented Sep 17, 2020

You'll have to make GcpCloudSqlAutoConfiguration and GcpStorageAutoConfiguration non-abstract for the tests to pass.
We've made them abstract to appease checkstyle, which no longer looks like a good tradeoff.

Thanks for the hint

According to the https://github.com/spring-cloud/spring-cloud-gcp/blob/master/CONTRIBUTING.adoc I'm supposed to run

$ bash util/update-license-year.sh

this is failing for me with:

Updating year to 2020 on files different from HEAD
xargs: illegal option -- -

@gabac - No worries about it; we probably have to fix the instructions...

Thanks for your contribution, looks good!

Copy link
Contributor

@meltsufin meltsufin left a comment

Choose a reason for hiding this comment

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

Thanks for the contricution @gabac!
It's good to go in pending @elefeint's verification of the integration tests.

@gabac
Copy link
Contributor Author

gabac commented Sep 17, 2020

Happy to do this 👍

Copy link
Contributor

@elefeint elefeint left a comment

Choose a reason for hiding this comment

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

After adding in the master build fixes, this PR passes with flying colors.

@elefeint elefeint merged commit 3739a52 into spring-attic:master Sep 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

GraalVM Support Switch spring-cloud-gcp autoconfigurations to proxyBeanMethods=false
4 participants