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

Atheriel remove connection close workaround #332

Conversation

schloerke
Copy link
Collaborator

Fixes: #318

Added commit commenting on tcp test path originally created when bug was found.

@schloerke
Copy link
Collaborator Author

test failure is not due to PR.

@schloerke schloerke merged commit aefb2e7 into rstudio:master Nov 5, 2018
@schloerke schloerke deleted the atheriel-remove-connection-close-workaround branch November 5, 2018 15:41
@atheriel
Copy link
Contributor

atheriel commented Nov 6, 2018

I notice that the rebase removed me as the author of the patch, which is a bit disappointing. Was that on purpose?

@schloerke schloerke restored the atheriel-remove-connection-close-workaround branch November 6, 2018 18:09
schloerke added a commit to schloerke/plumber that referenced this pull request Nov 6, 2018
* master:
  atheriel - remove connection close workaround (rstudio#318, rstudio#332)
@schloerke
Copy link
Collaborator Author

Hi @atheriel .

I am SO sorry about that. 100% my fault. I was trying to keep all of the changes made regarding the TCP connections under one PR merge. I thought squash and merge would credit all authors in the PR. It didn't.

I'd be happy to merge the branch in without a squash. It'd show up as work done in parallel (have 0 effective code change), but you'd be a direct author in the commit history. I don't feel I can't force push on master given plumber's traction.

schloerke added a commit to schloerke/plumber that referenced this pull request Nov 6, 2018
* master:
  atheriel - remove connection close workaround (rstudio#318, rstudio#332)
@schloerke
Copy link
Collaborator Author

@atheriel I just found https://help.github.com/articles/creating-a-commit-with-multiple-authors/ and will be using the feature in the future.

@atheriel
Copy link
Contributor

atheriel commented Nov 6, 2018

@schloerke I'd never realized squashing had this behaviour before, either. Seems like a bit of an anti-feature. Glad you understand :)

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.

None yet

2 participants