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.

Sign up for GitHub

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

Jump to bottom

Remove sealed module #543

Merged
merged 1 commit into from
Mar 25, 2025
Merged

Remove sealed module #543

merged 1 commit into from
Mar 25, 2025
+28 −88

Conversation

pvdrz
Copy link
Contributor

@pvdrz pvdrz commented Mar 24, 2025

All the assertions exposed by this module can be written using const for a while.

@pvdrz pvdrz force-pushed the remove-sealed-mod branch 2 times, most recently from ebf4948 to c5d07a5 Compare March 24, 2025 22:24
@pvdrz
Copy link
Contributor Author

pvdrz commented Mar 24, 2025

This probably needs the skip changelog check label

newAM
newAM requested changes Mar 24, 2025
View reviewed changes
Copy link
Member

@newAM newAM left a comment

Choose a reason for hiding this comment

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

These asserts were compile time before, I don't think we should replace them with runtime equivalents.

@Dirbaio
Copy link
Member

Dirbaio commented Mar 24, 2025

I think you can do const { assert!(N > 0) } to make it fail at compile time.

@pvdrz
Remove sealed module

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
Learn about vigilant mode
Loading
Loading status checks…
18446d7
@pvdrz pvdrz force-pushed the remove-sealed-mod branch from c5d07a5 to 18446d7 Compare March 24, 2025 23:26
@pvdrz
Copy link
Contributor Author

pvdrz commented Mar 24, 2025

oh sorry. I totally overlooked that fact. I force-pushed to include all the const{ .. } blocks as @Dirbaio suggested.

newAM
newAM approved these changes Mar 25, 2025
View reviewed changes
Copy link
Member

@newAM newAM left a comment

Choose a reason for hiding this comment

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

Thanks!

@newAM newAM added this pull request to the merge queue Mar 25, 2025
Merged via the queue into rust-embedded:main with commit b824515 Mar 25, 2025
22 checks passed
@newAM newAM mentioned this pull request Mar 25, 2025
Open
14 tasks
@pvdrz pvdrz deleted the remove-sealed-mod branch March 25, 2025 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@newAM newAM

Assignees
No one assigned
Labels
skip-changelog
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@pvdrz @Dirbaio @newAM