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

Rename shutdown() to closewrite() #41995

Merged
merged 1 commit into from
Aug 26, 2021
Merged

Conversation

c42f
Copy link
Member

@c42f c42f commented Aug 25, 2021

shutdown() was introduced in #40783. It's exported from Base, but has not yet been released so renaming is fair game.

The name closewrite() was suggested in #24526 and

  • Has a good analogy to close(), so people are likely to be able to
    guess what it means.
  • Is more specific to IO (conversely, it's easy to imagine shutdown()
    being wanted for any number of things unrelated to closing the write
    side of an IO stream).

See also discussion at #40783

@c42f c42f requested a review from vtjnash August 25, 2021 01:46
@c42f c42f force-pushed the cjf/rename-shutdown-closewrite branch from 6d9e36d to c039f53 Compare August 25, 2021 04:37
This name was suggested in #24526 and

* Has a good analogy to close(), so people are likely to be able to
  guess what it means.
* Is more specific to IO (conversely, it's easy to imagine shutdown()
  being wanted for any number of things unrelated to closing the write
  side of an IO stream).
@c42f c42f force-pushed the cjf/rename-shutdown-closewrite branch from c039f53 to b44f4a9 Compare August 25, 2021 04:38
@DilumAluthge
Copy link
Member

DilumAluthge commented Aug 25, 2021

Can you hold off on merging this? I think I've found a bug on Linux x86_64 that was introduced by #40783. I'm almost done with the bisect.

If the bisect does indeed point to #40783, I'm going to make a new revert PR for #40783, which will be easier if this PR has not been merged.

@c42f
Copy link
Member Author

c42f commented Aug 25, 2021

Will do. The tests failed here in any case because I'd missed the a case of shutdown in the docs.

@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Aug 25, 2021
@c42f
Copy link
Member Author

c42f commented Aug 26, 2021

Can you hold off on merging this

Ref: #42005

@c42f c42f added the io Involving the I/O subsystem: libuv, read, write, etc. label Aug 26, 2021
@DilumAluthge
Copy link
Member

Can you hold off on merging this

Ref: #42005

Yep, we're no longer planning on a revert, so there's no issue here.

@vtjnash vtjnash merged commit c298e4e into master Aug 26, 2021
@vtjnash vtjnash deleted the cjf/rename-shutdown-closewrite branch August 26, 2021 15:44
@KristofferC
Copy link
Member

KristofferC commented Aug 31, 2021

As a heads up, IOExtras exports a symbol with the same name so anything doing using IOExtras (like HTTP.jl) does currently not work.

@DilumAluthge DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label Sep 6, 2021
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
This name was suggested in JuliaLang#24526 and

* Has a good analogy to close(), so people are likely to be able to
  guess what it means.
* Is more specific to IO (conversely, it's easy to imagine shutdown()
  being wanted for any number of things unrelated to closing the write
  side of an IO stream).
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
This name was suggested in JuliaLang#24526 and

* Has a good analogy to close(), so people are likely to be able to
  guess what it means.
* Is more specific to IO (conversely, it's easy to imagine shutdown()
  being wanted for any number of things unrelated to closing the write
  side of an IO stream).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
io Involving the I/O subsystem: libuv, read, write, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants