-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Separate MessagePackHubProtocol into its own package #25253
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
Conversation
Don't you need a separate .javaproj and dependencies file etc.? |
@BrennanConroy @halter73 is the goal to get two separate .jars, or two separate packages within the signalr .jar we already produce? Still trying to figure out how to do the second, if that's what we need (this does the first) |
The only .javaproj in the folder is |
That's a small hack, it's actually for tests and the package.
This. |
10f4772
to
3de4db5
Compare
3de4db5
to
b9244b8
Compare
New internal build: https://dev.azure.com/dnceng/public/_build/results?buildId=795120&view=results @BrennanConroy @halter73 PTAL |
@Pilchie asking for this to go in for rc1 - it separates MessagePack into its own package, matching what we do in the C# & TypeScript clients. Also allows us to remove unneeded dependencies from each package |
Hello human! Please make sure you've included the Shiproom Template in a comment or (preferably) the PR description. Also, make sure this PR is not marked as a draft and is ready-to-merge. |
.../clients/java/signalr/core/src/main/java/com/microsoft/signalr/HttpHubConnectionBuilder.java
Outdated
Show resolved
Hide resolved
src/SignalR/clients/java/signalr/core/signalr.client.java.core.javaproj
Outdated
Show resolved
Hide resolved
@wtgodbe haven't looked at it at all yet |
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.
We can figure out the VSCode test stuff later
Well, I'd rather get this in RC1 than later, so... Approved for RC1 pending CI completion if merged before 10am Pacific on 2020-09-01. |
dependencies { | ||
implementation project(':core') | ||
compile 'org.msgpack:msgpack-core:0.8.20' | ||
compile 'org.msgpack:jackson-dataformat-msgpack:0.8.20' |
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.
When I try using compile
in build.gradle it warns "compile
is deprecated; replace with implementation
"
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.
What command did you run to produce that warning? I don't see it locally w/ compileJava
or createPackage
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.
src/SignalR/clients/java/signalr/test/signalr.client.java.Tests.javaproj
Show resolved
Hide resolved
Hello human! Please make sure you've included the Shiproom Template in a comment or (preferably) the PR description. Also, make sure this PR is not marked as a draft and is ready-to-merge. |
Removed approval since the deadline passed. Please update the shiproom template for it. |
Approved for RC1. |
1 similar comment
Approved for RC1. |
Resolves #25087
Internal build: https://dev.azure.com/dnceng/internal/_build/results?buildId=795545&view=results
Todo - update this https://github.com/dotnet/aspnetcore-internal/blob/master/docs/release/ReleaseActivities.md#step-0-before-the-release, and this https://github.com/dotnet/aspnetcore-internal/blob/master/docs/release/ReleaseActivities.md#what-gets-published