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

actix-utils: Remove unsound custom Cell as well #161

Merged
merged 1 commit into from
Jul 22, 2020

Conversation

JohnTitor
Copy link
Member

PR Type

Bug Fix/Refactor

PR Checklist

Check your PR fulfills the following:

  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • A changelog entry has been made for the appropriate packages.

Overview

The changes are the almost same as #158, but it's for actix-utils.
Fixes #160
cc @Shnatsel

@codecov
Copy link

codecov bot commented Jul 20, 2020

Codecov Report

Merging #161 into master will decrease coverage by 0.02%.
The diff coverage is 96.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #161      +/-   ##
==========================================
- Coverage   60.50%   60.47%   -0.03%     
==========================================
  Files          74       73       -1     
  Lines        4790     4782       -8     
==========================================
- Hits         2898     2892       -6     
+ Misses       1892     1890       -2     
Impacted Files Coverage Δ
actix-utils/src/either.rs 0.00% <ø> (ø)
actix-utils/src/stream.rs 0.00% <ø> (ø)
actix-utils/src/time.rs 80.00% <83.33%> (ø)
actix-utils/src/condition.rs 93.61% <100.00%> (+0.13%) ⬆️
actix-utils/src/mpsc.rs 76.31% <100.00%> (+0.31%) ⬆️
actix-utils/src/oneshot.rs 94.87% <100.00%> (+0.18%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d0bd7c...3f733cd. Read the comment docs.

@Shnatsel
Copy link
Contributor

Looks good to me on the surface. I can't claim to understand that's going on, though.

Thanks for the swift fix!

@JohnTitor JohnTitor requested review from a team July 20, 2020 13:37
Copy link

@cetra3 cetra3 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@robjtede robjtede mentioned this pull request Jul 21, 2020
15 tasks
Copy link
Member

@robjtede robjtede left a comment

Choose a reason for hiding this comment

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

nice job, we should follow up soon and get all the unsafe blocks documented and debug_assert-ed

@robjtede robjtede merged commit 0dca1a7 into actix:master Jul 22, 2020
@JohnTitor JohnTitor deleted the cell branch July 22, 2020 00:15
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.

Custom Cell implementation is unsound
4 participants