search

spring-cloud / spring-cloud-circuitbreaker

Spring Cloud Circuit Breaker API and Implementations
Apache License 2.0
329 stars 110 forks source link

move to a separate file #135

Closed wind57 closed 2 years ago

wind57 commented 2 years ago

I'd like to become a contributor in this project also and as usual, my understanding of what it does and how starts from a few pull requests refactoring minor things. As such this PR does two things only:

spencergibb commented 2 years ago

Can you explain why this "moves SpringRetryConfig to a separate class?"

wind57 commented 2 years ago

it's a public class residing in another public class, with package private methods. To me, it is a lot easier to read code. Making it an inner class does not provide any benefit at all, and in contrast defeats the purpose of an inner class.

imo, moving it to a separate file, makes the holder (SpringRetryConfigBuilder) a lot easier to reason about.

codecov[bot] commented 2 years ago

Codecov Report

Merging #135 (83ff489) into main (915dde1) will increase coverage by 0.58%. The diff coverage is 94.44%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #135      +/-   ##
============================================
+ Coverage     77.56%   78.15%   +0.58%     
- Complexity       81       91      +10     
============================================
  Files            14       15       +1     
  Lines           361      357       -4     
  Branches         12       12              
============================================
- Hits            280      279       -1     
+ Misses           77       74       -3     
  Partials          4        4
Impacted Files Coverage Δ
...breaker/springretry/SpringRetryCircuitBreaker.java 100.00% <ø> (ø)
...tbreaker/springretry/SpringRetryConfigBuilder.java 73.91% <ø> (-2.28%) :arrow_down:
.../circuitbreaker/springretry/SpringRetryConfig.java 93.75% <93.75%> (ø)
.../springretry/SpringRetryCircuitBreakerFactory.java 83.33% <100.00%> (-1.29%) :arrow_down:

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 915dde1...83ff489. Read the comment docs.