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

Upgrade to Spring Boot 2.1 #825

Merged
merged 8 commits into from
Jan 29, 2019
Merged

Upgrade to Spring Boot 2.1 #825

merged 8 commits into from
Jan 29, 2019

Conversation

tgianos
Copy link
Contributor

@tgianos tgianos commented Jan 29, 2019

Updates to Spring Framework 5.1 transitively and Spring Cloud Greenwich.RELEASE which is the cloud release which supports Boot 2.1

@tgianos tgianos added this to the 4.0.0 milestone Jan 29, 2019
@tgianos tgianos self-assigned this Jan 29, 2019
@tgianos tgianos requested a review from mprimi January 29, 2019 18:37
build.gradle Outdated
@@ -91,7 +91,7 @@ configure(javaProjects) {
dependency("commons-httpclient:commons-httpclient:3.1")
dependency("commons-io:commons-io:2.6")
dependency("commons-validator:commons-validator:1.6")
dependencySet(group: "io.grpc", version: "1.12.0") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this really necessary? Do we run into some kind of trouble by leaving it higher than recommended?

If i remember correctly, we were getting some very weird concurrency bug that would corrupt the network buffers, it was a known issue, and upgrading to 1.12 fixed it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know that it is. You didn't mention that issue yesterday when I asked. It was easier to back down than battle dependency hell internally with the versions but I can remove it and deal with that if there is some functional issue that 1.12 fixed. Since the build passed I didn't think it was an issue. I'll remove this and see if I can resolve the internal versioning.

@codecov
Copy link

codecov bot commented Jan 29, 2019

Codecov Report

Merging #825 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #825      +/-   ##
============================================
+ Coverage     88.46%   88.46%   +<.01%     
  Complexity     2958     2958              
============================================
  Files           361      361              
  Lines         11855    11856       +1     
  Branches        790      790              
============================================
+ Hits          10488    10489       +1     
  Misses         1011     1011              
  Partials        356      356
Impacted Files Coverage Δ Complexity Δ
.../web/resources/writers/DefaultDirectoryWriter.java 94.73% <100%> (+0.04%) 19 <0> (ø) ⬇️
...genie/web/jobs/workflow/impl/InitialSetupTask.java 93.91% <100%> (ø) 14 <0> (ø) ⬇️
...web/services/impl/JobSpecificationServiceImpl.java 81.98% <100%> (ø) 23 <0> (ø) ⬇️
...enie/web/services/impl/DiskJobFileServiceImpl.java 91.3% <100%> (ø) 14 <0> (ø) ⬇️
...x/genie/web/configs/GenieApiAutoConfiguration.java 92.68% <100%> (ø) 13 <0> (ø) ⬇️
...t/web/configs/IntegrationTestingConfiguration.java 50% <0%> (-16.67%) 3% <0%> (ø)
...lient/interceptors/ResponseMappingInterceptor.java 78.94% <0%> (+10.52%) 4% <0%> (ø) ⬇️

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 9139e04...60c6194. Read the comment docs.

@coveralls
Copy link

coveralls commented Jan 29, 2019

Coverage Status

Coverage increased (+0.02%) to 91.481% when pulling 60c6194 on tgianos:boot21 into 9139e04 on Netflix:master.

@tgianos
Copy link
Contributor Author

tgianos commented Jan 29, 2019

@chali Backing Gradle back down here until Gradle Git Publish Issue is addressed

@tgianos
Copy link
Contributor Author

tgianos commented Jan 29, 2019

@twicksell FYI in case you have any issues here

@twicksell
Copy link

@tgianos lgtm 👍

@tgianos tgianos merged commit 3c28570 into Netflix:master Jan 29, 2019
@tgianos tgianos deleted the boot21 branch January 29, 2019 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants