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

updating signalr js package readme #22129

Merged
merged 3 commits into from
Jun 12, 2020
Merged

Conversation

bradygaster
Copy link
Member

per the doc issue reported here, this update makes it more up-front which client customers should use, when their only knowledge of SignalR Is from perusing NPM packages, or who find their way here via OG SignalR docs.

wasn't sure which branch to suggest as upstream - goal here is simply to update the npm readme for the pre-@microsoft/signalr era.

@ghost ghost added the area-signalr Includes: SignalR clients and servers label May 22, 2020
@ghost ghost added this to PR Open in Servicing Status May 22, 2020
@BrennanConroy BrennanConroy added the Servicing-consider Shiproom approval is required for the issue label May 22, 2020
@ghost
Copy link

ghost commented May 22, 2020

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.

@ghost ghost moved this from PR Open to Submitted to Shiproom in Servicing Status May 22, 2020
@BrennanConroy BrennanConroy added this to the 2.1.x milestone May 22, 2020
@leecow leecow added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels May 26, 2020
@ghost ghost moved this from Submitted to Shiproom to Approved by Shiproom in Servicing Status May 26, 2020
@leecow leecow modified the milestones: 2.1.x, 2.1.20 May 26, 2020
@BrennanConroy
Copy link
Member

BrennanConroy commented Jun 12, 2020

We need to update the patch config

<PackagesInPatch>

Add @aspnet/signalr

@BrennanConroy BrennanConroy requested a review from a team June 12, 2020 21:10
@wtgodbe
Copy link
Member

wtgodbe commented Jun 12, 2020

@BrennanConroy we need to re-ship this package with the updated readme?

@BrennanConroy
Copy link
Member

Yes, we're trying to update the README on npm which needs a published package with the update.

@BrennanConroy
Copy link
Member

???

@dougbu
Copy link
Member

dougbu commented Jun 12, 2020

Well, that's not good. 21 commits is much more than I expected. Did you perhaps rebase on release/3.1❔ Or is this just another GitHub UI oddity❔

@BrennanConroy
Copy link
Member

Well, that's not good. 21 commits is much more than I expected. Did you perhaps rebase on release/3.1❔ Or is this just another GitHub UI oddity❔

Neither which is why this is super annoying.

Copy link
Member

@dougbu dougbu left a comment

Choose a reason for hiding this comment

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

this is super annoying.

Yes

eng/PatchConfig.props Show resolved Hide resolved
@BrennanConroy
Copy link
Member

Fixed

@dougbu
Copy link
Member

dougbu commented Jun 12, 2020

Was the issue an incomplete rebase i.e. having conflicts when pushing❔

@BrennanConroy
Copy link
Member

Having a different remote that was ~21 commits behind :/

@dougbu
Copy link
Member

dougbu commented Jun 12, 2020

Please ping when checks complete and you've confirmed the npm package is created correctly. @wtgodbe or I will get it in.

@BrennanConroy
Copy link
Member

Npm package created and build passed 👍

@wtgodbe wtgodbe merged commit 8dce530 into release/2.1 Jun 12, 2020
@wtgodbe wtgodbe deleted the signalr-2-1-readme-updates branch June 12, 2020 23:19
@ghost ghost moved this from Approved by Shiproom to Merged in Servicing Status Jun 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-signalr Includes: SignalR clients and servers Servicing-approved Shiproom has approved the issue
Projects
No open projects
Servicing Status
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

7 participants