Skip to content
This repository has been archived by the owner on Feb 23, 2021. It is now read-only.

React to SignalR changes #154

Merged
merged 1 commit into from
Nov 11, 2016
Merged

React to SignalR changes #154

merged 1 commit into from
Nov 11, 2016

Conversation

pranavkm
Copy link
Contributor

@muratg
Copy link
Contributor

muratg commented Nov 10, 2016

cc @anurse

@@ -173,6 +171,7 @@ Microsoft.Extensions.TaskCache.Sources,noship,exclude
Microsoft.Extensions.TypeNameHelper.Sources,noship,exclude
Microsoft.Extensions.WebEncoders,ship,include
Microsoft.Extensions.WebEncoders.Sources,noship,exclude
Microsoft.Extensions.WebSockets.Internal,noship,exclude
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this package listed twice?

Copy link
Contributor

Choose a reason for hiding this comment

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

nvm I misread

Copy link
Member

Choose a reason for hiding this comment

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

Noship?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a package with .Internal. We really shouldn't be publishing it to nuget.org

Copy link
Contributor

Choose a reason for hiding this comment

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

We have another .Internal package that ships I believe. I'd like @anurse to comment because he knows the latest on this.

Copy link
Contributor

Choose a reason for hiding this comment

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

It needs to ship along with SignalR, so it needs to be 'ship'. It's .Internal to indicate that it is not currently supported on it's own. It's only "supported" via using SignalR.

Copy link
Member

Choose a reason for hiding this comment

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

Thats what I thought

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By ship, we mean publish to nuget.org when 1.2.0 comes out. Does that sound right? If so, can we come up with a better name for this or make it a .Sources package?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, publish at 1.2.0. The big reason for the name is that we are already shipping a Microsoft.Extensions.WebSockets, so we'll have to merge the two when we actually come to ship it. I guess we could make it a sources package, or just pull the source in and not ship the package if @davidfowl 's cool with that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filed aspnet/SignalR#24 to track a better Id for those packages and \ or an alternate solution. I'll leave them as noship for now

@pranavkm
Copy link
Contributor Author

bump. Need to get this in to unblock dev

Microsoft.AspNetCore.Server.IISIntegration,ship,include
Microsoft.AspNetCore.Server.IntegrationTesting,noship,exclude
Microsoft.AspNetCore.Server.Kestrel,ship,include
Microsoft.AspNetCore.Server.Kestrel.Https,ship,include
Microsoft.AspNetCore.Server.WebListener,ship,include
Microsoft.AspNetCore.Session,ship,include
Microsoft.AspNetCore.SignalR.Messaging,noship,exclude
Microsoft.AspNetCore.SignalR.Redis,noship,exclude
Copy link
Member

Choose a reason for hiding this comment

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

Should be changed to ship

@@ -77,22 +77,20 @@ Microsoft.AspNetCore.Rewrite,ship,include
Microsoft.AspNetCore.Routing,ship,include
Microsoft.AspNetCore.Routing.Abstractions,ship,include
Microsoft.AspNetCore.Routing.DecisionTree.Sources,noship,exclude
Microsoft.AspNetCore.Sockets,noship,exclude
Copy link
Member

Choose a reason for hiding this comment

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

Also ship

@pranavkm
Copy link
Contributor Author

🆙 :date

Microsoft.AspNetCore.StaticFiles,ship,include
Microsoft.AspNetCore.TestHost,ship,exclude
Microsoft.AspNetCore.Testing,noship,exclude
Microsoft.AspNetCore.WebSockets,ship,include
Microsoft.AspNetCore.WebSockets.Internal,noship,exclude
Copy link
Member

Choose a reason for hiding this comment

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

Ship

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought we were planning on renaming these first: aspnet/SignalR#24

Copy link
Member

Choose a reason for hiding this comment

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

Does this affect anything right now? If not that's fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. Just makes sure we don't ignore the work item :)

@pranavkm pranavkm merged commit b39f07a into dev Nov 11, 2016
@pranavkm pranavkm deleted the prkrishn/signalr branch November 11, 2016 00:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants