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

Fix Future::on_completion #5181

Merged
merged 2 commits into from
Jan 21, 2022
Merged

Fix Future::on_completion #5181

merged 2 commits into from
Jan 21, 2022

Conversation

jbreams
Copy link
Contributor

@jbreams jbreams commented Jan 20, 2022

What, How & Why?

I noticed while working on something that Future::on_completion() was not working. I don't know how I missed this when I was doing this work earlier in the year, but it looks like I missed some test coverage here. I've fixed on_completion and added a bunch of tests for it.

There were also two other minor bugs in the future header that get fixed as well.

☑️ ToDos

  • 📝 Changelog update
  • 🚦 Tests (or not relevant)

@cla-bot cla-bot bot added the cla: yes label Jan 20, 2022
@jbreams jbreams requested a review from ironage January 20, 2022 22:20
Copy link
Contributor

@ironage ironage left a comment

Choose a reason for hiding this comment

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

Well, the good news is, if the compiler didn't catch this then for sure nobody was using it. 😄

@jbreams jbreams merged commit b595667 into master Jan 21, 2022
@jbreams jbreams deleted the jbr/fix_future_on_completion branch January 21, 2022 17:22
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants