-
Notifications
You must be signed in to change notification settings - Fork 44
[v0.2.0 Refactor] [Type-C] Remove static_cell dependency and improve context handling #668
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
[v0.2.0 Refactor] [Type-C] Remove static_cell dependency and improve context handling #668
Conversation
8c975f6 to
16aadd1
Compare
asasine
left a comment
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.
Looks functional so approved. All of my comments are therefore non-blocking.
&'static->&'ais a step for testability.- Adding methods to
Contextand getting rid of the external public static functions is a usability improvement. - Using
self.controllers()instead of passing a field as an arg to your own methods is hygeine and usability.
4064a52
4064a52 to
113f3af
Compare
…context handling - Removed `static_cell` dependency from Cargo.lock and Cargo.toml in examples and type-c-service. - Updated Type-C service methods to accept a reference to `intrusive_list::IntrusiveList` for better context management. - Modified various service methods to pass the controllers list where necessary, ensuring proper context usage. - Commented out unused code related to power policy channels and service initialization. - Adjusted the task closure to work with the updated service structure. - Enhanced error handling and logging throughout the service methods. - Initializing the service will now register all PD controllers before running the type-c task WIP
113f3af to
fb4a619
Compare
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.
Exposing intrusive_list::IntrusiveList as a part of the public API worries me a little bit as it is a bit not user friendly as an API. But if all the code owners for type C agrees that this is the most optimal way to do it, then it is fine.
@tullom Can you post this PR with a high-level description of why we are doing this refactoring on Zulip or maybe as a RFC on https://github.com/OpenDevicePartnership/governance/tree/main/rfc? Thanks.
Long-term I think there's a good chance we'll end up switching to slices or something similar. But I think this is a good incremental change towards that goal. |
Done, see Zulip thread here: |
static_celldependency from Cargo.lock and Cargo.toml in examples and type-c-service.intrusive_list::IntrusiveListfor better context management.