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

Service / Simplification Documentation and Release Prep #954

Merged
merged 5 commits into from
Aug 17, 2023
Merged

Conversation

scottf
Copy link
Contributor

@scottf scottf commented Aug 17, 2023

  • Documented Service Framework
  • Removed experimental tags from Service and Simplification
  • Fixed test flappers

@scottf scottf changed the title Service Api Documentation Service / Simplification Documentation and Release Prep Aug 17, 2023
@@ -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"); };
Copy link
Collaborator

@arondi arondi Aug 17, 2023

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 + "]");
Copy link
Collaborator

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);
Copy link
Collaborator

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
Copy link
Collaborator

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[]{
Copy link
Collaborator

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();
}

Copy link
Collaborator

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 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Name makes more sense

Copy link
Collaborator

@arondi arondi left a 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

@scottf scottf merged commit 6317568 into main Aug 17, 2023
1 check passed
@scottf scottf deleted the service-doc branch September 7, 2023 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants