Add FeignClientBuilder.Builder#customize(FeignBuilderCustomizer) (#499) #515
Conversation
| context.close(); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
copied from above, diff 83, 84, 88, 90
| ReflectionUtils.makeAccessible(factoryBeanField); | ||
| final FeignClientFactoryBean factoryBean = (FeignClientFactoryBean) ReflectionUtils.getField(factoryBeanField, | ||
| builder); | ||
| builder); |
There was a problem hiding this comment.
using IDEA style from spring-cloud-build-tools
There was a problem hiding this comment.
@Sam-Kruglov Thanks for submitting the PR. In general, it looks good. There are a few minor issues to address - please take a look at the comments. Also, please add your full name with @author tags to the javadocs of the classes you've changed. Apart from this, since this is not backwards incompatible, I'm thinking it would make sense to add it to the 2.2.x branch instead of master, so that it's available in the Hoxton release train as well (the easiest way of doing that would probably be to cherry-pick the commits against 2.2.x and create a new branch and PR).
|
|
||
| private boolean followRedirects = new Request.Options().isFollowRedirects(); | ||
|
|
||
| private List<FeignBuilderCustomizer> additionalCustomizers = new LinkedList<>(); |
There was a problem hiding this comment.
Why have you chosen LinkedList?
There was a problem hiding this comment.
Because we don't really need the indices here, but ok, I'll put array instead to keep things consistent.
| } | ||
|
|
||
| /** | ||
| * @param customizer applied in the same order as supplied here |
There was a problem hiding this comment.
Please add a description line on top of the javadoc and the @return tag.
There was a problem hiding this comment.
I think this is self-explanatory but sure
There was a problem hiding this comment.
I meant sth more like the {@link Builder} with the customizer added rather than just this.
Sam-Kruglov
force-pushed
the
feature/499_customizer
branch
from
3c71c48 to
85b724c
Compare
|
rerouted to 2.2.x |
Sam-Kruglov
force-pushed
the
feature/499_customizer
branch
from
85b724c to
7ada18f
Compare
There was a problem hiding this comment.
@Sam-Kruglov Thanks for adding the changes - just one small change to be made and it will be good to merge.
| } | ||
|
|
||
| /** | ||
| * @param customizer applied in the same order as supplied here |
There was a problem hiding this comment.
I meant sth more like the {@link Builder} with the customizer added rather than just this.
Sam-Kruglov
force-pushed
the
feature/499_customizer
branch
from
7ada18f to
ebc3f58
Compare
|
Fixes gh-499 |
fixes #499