Skip to content

Merge 2.1 into 3.1 #18369

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Jan 16, 2020
Merged

Merge 2.1 into 3.1 #18369

merged 10 commits into from
Jan 16, 2020

Conversation

jkotalik
Copy link
Contributor

@jkotalik jkotalik commented Jan 16, 2020 •
edited
Loading

Another attempt at #18347 as I think the merge went wrong.

@jkotalik jkotalik changed the title Jkotalik/merge31 Merge 2.1 into 3.1 Jan 16, 2020
Tratcher
Tratcher reviewed Jan 16, 2020
View reviewed changes
jkotalik
jkotalik commented Jan 16, 2020
View reviewed changes
src/SignalR/server/Core/src/HubConnectionContext.cs
@@ -318,6 +318,7 @@ private ValueTask TryWritePingAsync()
return default;
}

// TODO: cancel?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@BrennanConroy anything to be concerned about here?

Copy link
Member

Choose a reason for hiding this comment

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

Haha, not really

@jkotalik
Copy link
Contributor Author

Taking in favor of #18347, yeah?

@dougbu
Copy link
Contributor

dougbu commented Jan 16, 2020

This is confusing. Why not force push the #18347 branch and keep all the comments in one place?

@jkotalik
Copy link
Contributor Author

Because I wanted to make sure this PR was correct before squashing over @BrennanConroy . At this point, as it is already building, I'd rather just let it be and let it pass.

Also, many of the comments were talking about things that weren't relevant to the merge (things that existed in 3.1 that weren't changed).

@dougbu
Copy link
Contributor

dougbu commented Jan 16, 2020

  1. We're not going to squash anything -- this will be merged 😈
  2. 🆗 I'll review this one and ignore [automated] Merge branch 'release/2.1' => 'release/3.1' #18347
  3. Please close [automated] Merge branch 'release/2.1' => 'release/3.1' #18347 as soon as you get relatively confident in the validation builds

@dougbu
Copy link
Contributor

dougbu commented Jan 16, 2020

Templates failures are discussed elsewhere. This is looking good so far.

@jkotalik
Copy link
Contributor Author

So what should I do about the template failures? I'd like to get this in stat...

@BrennanConroy
Copy link
Member

Looks like templates will be fixed once #18380 goes in

@BrennanConroy
Copy link
Member

All but one of the templates tests passed on rerun: BlazorServerTemplateWorks_IndividualAuth(useLocalDB: True)

Looks unrelated to previous templates issues, is it a flaky test?

@rynowak
Copy link
Member

rynowak commented Jan 16, 2020

I'm rerunning the template tests.

@JunTaoLuo JunTaoLuo merged commit bc7135c into release/3.1 Jan 16, 2020
@JunTaoLuo JunTaoLuo deleted the jkotalik/merge31 branch January 16, 2020 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tratcher Tratcher Tratcher approved these changes

@JunTaoLuo JunTaoLuo JunTaoLuo approved these changes

@dougbu dougbu dougbu approved these changes

@BrennanConroy BrennanConroy BrennanConroy approved these changes

@analogrelay analogrelay Awaiting requested review from analogrelay

@halter73 halter73 Awaiting requested review from halter73 halter73 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@jkotalik @dougbu @BrennanConroy @rynowak @Tratcher @JunTaoLuo @dotnet-bot