Skip to content

Move protocol implementation into new package#81

Merged
testforstephen merged 5 commits into
masterfrom
jinbo_refactor
Nov 2, 2017
Merged

Move protocol implementation into new package#81
testforstephen merged 5 commits into
masterfrom
jinbo_refactor

Conversation

@testforstephen

Copy link
Copy Markdown
Contributor

Signed-off-by: Jinbo Wang jinbwan@microsoft.com

Signed-off-by: Jinbo Wang <jinbwan@microsoft.com>
Signed-off-by: Jinbo Wang <jinbwan@microsoft.com>
@testforstephen

Copy link
Copy Markdown
Contributor Author

This PR is related to #50. Move it to a new package, but still keep it in core.jar.

@akaroml

akaroml commented Oct 17, 2017

Copy link
Copy Markdown
Member

Let's release this in 0.3.0.

this.terminateSession = true;
}

public boolean isTerminating() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Quite curious about the different about
isTerminating and "terminateSession```

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

duplicate. Remove isTerminating method.

String header = String.format("Content-Length: %d%s", jsonBytes.length, TWO_CRLF);
byte[] headerBytes = header.getBytes(PROTOCOL_ENCODING);

byte[] data = new byte[headerBytes.length + jsonBytes.length];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sounds some duplication? Should be ByteBuffer?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nice catch. fixed.

yaohaizh
yaohaizh previously approved these changes Nov 2, 2017
Signed-off-by: Jinbo Wang <jinbwan@microsoft.com>
@testforstephen
testforstephen merged commit f612bdf into master Nov 2, 2017
@testforstephen
testforstephen deleted the jinbo_refactor branch November 2, 2017 07:31
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.

3 participants