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 pool macro warnings. #456

Merged
merged 2 commits into from
Feb 27, 2024

Conversation

reitermarkus
Copy link
Member

@reitermarkus reitermarkus commented Feb 19, 2024

The allow attributes are not needed:

The static variable defined in the singleton method is never exposed, so the name doesn't need to be the same as the pool.

The name of the struct should simply follow the standard naming conventions, the examples have been updated to make this clear this should be PascalCase.

Properly fixes #411 and #454.

@reitermarkus reitermarkus force-pushed the fix-pool-macro-warnings branch 8 times, most recently from d8c8dc2 to adea799 Compare February 19, 2024 10:46
@reitermarkus reitermarkus requested a review from a team February 19, 2024 10:52
@Dirbaio
Copy link
Member

Dirbaio commented Feb 19, 2024

The static variable defined in the singleton method is never exposed, so the name doesn't need to be the same as the pool.

it is "exposed" as a symbol name. it's sometimes helpful to debug memory usage. If they're all named POOL you can't know which is which anymore.

@reitermarkus
Copy link
Member Author

@Dirbaio, okay, makes sense. I have reverted that change and added a note.

Dirbaio
Dirbaio previously approved these changes Feb 25, 2024
@Dirbaio
Copy link
Member

Dirbaio commented Feb 25, 2024

clippy seems unhappy

@reitermarkus
Copy link
Member Author

Right, I used the wrong lint name …

@reitermarkus reitermarkus added this pull request to the merge queue Feb 27, 2024
Merged via the queue into rust-embedded:main with commit 483862b Feb 27, 2024
22 checks passed
@reitermarkus reitermarkus deleted the fix-pool-macro-warnings branch February 27, 2024 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

style and clippy
2 participants