Skip to content

Conversation

@mrkn
Copy link
Contributor

@mrkn mrkn commented Mar 10, 2024

As we discussed with @vtjnash in PR #53341, it might be useful to introduce the throw option in the wait function for Task. If throw=false is specified, wait behaves like _wait; it prevents throwing a TaskFailedException.

@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Mar 11, 2024
@IanButterworth IanButterworth merged commit dcfad21 into JuliaLang:master Mar 11, 2024
@mrkn mrkn deleted the add_throw_kw_in_wait branch March 12, 2024 00:51
@inkydragon inkydragon removed the merge me PR is reviewed. Merge when all tests are passing label Mar 12, 2024
@MasonProtter
Copy link
Contributor

@mrkn would you mind adding a docsting? Thanks for adding this, I just ran into wanting it today!

@mrkn
Copy link
Contributor Author

mrkn commented Mar 13, 2024

@MasonProtter Sure, I've already modified the docstring in condition.jl in this pull request (here). If there are any other docstrings that need to be updated, please let me know.

@MasonProtter
Copy link
Contributor

Oh, I see, sorry for the noise then, I should have looked more carefully at the other changed file 🤦

@fredrikekre fredrikekre added the needs news A NEWS entry is required for this change label Mar 13, 2024
vtjnash pushed a commit that referenced this pull request Apr 16, 2024
Small follow-up to #53685, since
having critical documentation for a specific method be only mentioned in
a catch-all docstring isn't quite as helpful as it could be (e.g. when
looking at inline documentation with LSP, where it happens to know that
the variable being passed is a `Task`). In addition, this also documents
that `wait` on a `Task` will throw if given `current_task`.

I've also split the function into a prototype and the actual
implementation, so that the same "more specific docs at the method
definition" transform can be applied for `GenericCondition` too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs news A NEWS entry is required for this change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants