Implement RotatingServerAdvice enhancements #3028
Conversation
| */ | ||
| KeyDirectory getCurrent(); | ||
|
|
||
| } |
|
|
||
| /** | ||
| * | ||
| * @return the current {@link KeyDirectory} |
There was a problem hiding this comment.
I don't think this will pass checkstyle. Description needed.
|
@dturanski Please rebase and re-push; I have fixed the .travis.yml for this branch. |
|
Merged as 03b6316 with minor formatting fixes. |
| * Implementations can reconfigure the message source before and/or after | ||
| * a poll. | ||
| */ | ||
| public interface RotationPolicy { |
There was a problem hiding this comment.
We have to revert this change. It is fully blown breaking change in the point version.
I'm OK with a decoupling in the current 5.2 (preparing some polishing PR), but I'm against any breaking changes for public API in the point release.
Does it make sense, @garyrussell , @dturanski ?
Thanks
There was a problem hiding this comment.
Technically you're right. But I think it would be ok if we default it. Then it won't break anything. I'd be surprised if anyone else has actually implemented this interface.
There was a problem hiding this comment.
We just can't guess from here. This interface is public in this place, therefore moving it somewhere else is a breaking change.
The first rule: don't break any public API in the point release!
There was a problem hiding this comment.
I see, because I moved it from the inner class. Yes that is a breaking change.
There was a problem hiding this comment.
Good. So, @garyrussell , your turn now and then we will make a final decision!
|
Yes; I had a brain fart yesterday. But, we do need to satisfy David's extensibility needs. |
Right, so we leave it fixed in |
|
Reverted: f7032c7 |
Resolves #3027
Create
AbstractStandardRotationPolicysubclasses implementonRotation(MessageSource source).Pull dependent inner classes from RotatingServerAdvice to break circular dependencies.