-
Notifications
You must be signed in to change notification settings - Fork 221
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
Remove get_
prefix from functions
#2528
Conversation
|
@@ -4,7 +4,7 @@ use core::{cell::UnsafeCell, mem::MaybeUninit}; | |||
|
|||
use embassy_executor::{raw, SendSpawner}; | |||
use esp_hal::{ | |||
get_core, | |||
core, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we drop core()
in favour of Cpu::current()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just in this file, or in general?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general I think, I don't know if Rust can cope with a function core
and the core
crate together. Though I guess if it can, we can leave it and come back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay will do so in another PR, this one is a bit of a beast and I don't really want to touch it if CI is happy and I don't need to 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make get_core
private and use Cpu::current()
everywhere (though we don't have to do that here if we can get away with it).
188f640
to
bb14edc
Compare
get_
prefix from functions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks
bb14edc
to
06f7d17
Compare
I've skipped
xtensa-lx
as I don't think we really have any reason to publish a new version of that currently, but can update it as well if requested. Otherwise, I think I got them all (or most of them, at least 😅). Unfortunately had to touch a ton of files in the process, but oh well.Closes #2524