-
Notifications
You must be signed in to change notification settings - Fork 155
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
Service / Simplification Documentation and Release Prep #954
Conversation
scottf
commented
Aug 17, 2023
•
edited
Loading
edited
- Documented Service Framework
- Removed experimental tags from Service and Simplification
- Fixed test flappers
@@ -459,39 +459,39 @@ public List<String> getChanges(ConsumerConfiguration serverCc) { | |||
ConsumerConfigurationComparer serverCcc = new ConsumerConfigurationComparer(serverCc); | |||
List<String> changes = new ArrayList<>(); | |||
|
|||
if (deliverPolicy != null && deliverPolicy != serverCcc.getDeliverPolicy()) { changes.add("deliverPolicy"); }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra semi-colons unnecessary, good to remove.
@@ -75,10 +75,10 @@ public static String validateConsumerName(String s, boolean required) { | |||
public static String validatePrefixOrDomain(String s, String label, boolean required) { | |||
return _validate(s, required, label, () -> { | |||
if (s.startsWith(DOT)) { | |||
throw new IllegalArgumentException(label + " cannot start with `.` [" + s + "]"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing to quote from tic
public Discovery(Connection conn) { | ||
this(conn, -1, -1); | ||
this(conn, 0, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes more sense based on the comparison for < 1
{ | ||
//Ignore | ||
} | ||
Thread.sleep(250); // should get more pings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Flappers fix?
@@ -159,12 +159,12 @@ public void testServiceWorkflow() throws Exception { | |||
InfoResponse infoResponse2 = service2.getInfoResponse(); | |||
StatsResponse statsResponse1 = service1.getStatsResponse(); | |||
StatsResponse statsResponse2 = service2.getStatsResponse(); | |||
EndpointResponse[] endpointResponseArray1 = new EndpointResponse[]{ | |||
EndpointStats[] endpointStatsArray1 = new EndpointStats[]{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good naming change to match
public byte[] getData() { | ||
return message.getData(); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Glad to get rid of these unnecessary methods. Makes it clearer/simpler to use/understand.
*/ | ||
public class EndpointResponse implements JsonSerializable { | ||
public class EndpointStats implements JsonSerializable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name makes more sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good changes, thanks for the docs. LGTM
…nd simplification
…nd simplification