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

Replace depreciated atomic-polyfill crate with portable-atomic #9

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ImTheSquid
Copy link

atomic-polyfill has been depreciated and is causing problems in my ESP32-S3 build system. portable-atomic is the widely-accepted replacement and I have updated the library to use it.

CHANGELOG.md Outdated Show resolved Hide resolved
Cargo.toml Outdated
@@ -1,21 +1,16 @@
[package]
name = "atomic-pool"
version = "1.0.1"
version = "1.0.2"
Copy link
Member

Choose a reason for hiding this comment

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

this should be 2.0.0, since this is a breaking change.

Cargo.toml Outdated
as-slice-01 = { package = "as-slice", version = "0.1.5" }
as-slice-02 = { package = "as-slice", version = "0.2.1" }
portable-atomic = { version = "1.7.0", features = ["critical-section"] }
Copy link
Member

Choose a reason for hiding this comment

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

do not unconditionally enable critical-section, the user might want to use unsafe-assume-single-core instead which is slightly faster in single-core chips.

I'd add two features that forward to portable-atomic:

[features]
portable-atomic-critical-section = ["portable-atomic/critical-section"]
portable-atomic-unsafe-assume-single-core = ["portable-atomic/unsafe-assume-single-core"]

categories = ["embedded", "no-std", "concurrency", "memory-management"]

[features]
default = ["portable-atomic-critical-section"]
Copy link
Member

Choose a reason for hiding this comment

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

don't make this default. all it takes is one lib using atomic-pool and forgetting default-features=false and the user can't use unsafe-assume-single-core anymore.

Copy link
Author

Choose a reason for hiding this comment

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

The library doesn't compile without at least one of those enabled. Should I default to the other one or inject a compilation error if none are enabled?

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.

2 participants