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

Mark lots of enums #[non_exhaustive] #124

Merged
merged 5 commits into from
Apr 16, 2024
Merged

Mark lots of enums #[non_exhaustive] #124

merged 5 commits into from
Apr 16, 2024

Conversation

faern
Copy link
Member

@faern faern commented Apr 12, 2024

For the upcoming 0.7.0 release we have a lot of breaking changes due to adding variants to enums. Lots of these enums are probably still not exhaustive. To mitigate having to make breaking changes for small stuff in the future, I mark them as #[non_exhaustive]. If you think any of these actually are fully exhaustive now, and/or if you think an addition to one of them actually should be a breaking change, then please give that as feedback.

I want this merged before the 0.7.0 release.


This change is Reviewable

@faern faern requested a review from dlon April 12, 2024 15:12
@faern faern mentioned this pull request Apr 12, 2024
Copy link
Member

@dlon dlon left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved


src/service.rs line 950 at r1 (raw file):

/// Enum describing the reason of a SessionChange event
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
#[non_exhaustive]

This seems to be exhaustive right now.

@faern faern force-pushed the mark-enums-non_exhaustive branch from 37df250 to f215e4a Compare April 16, 2024 07:32
Copy link
Member Author

@faern faern left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 of 3 files reviewed, all discussions resolved (waiting on @dlon)


src/service.rs line 950 at r1 (raw file):

Previously, dlon (David Lönnhager) wrote…

This seems to be exhaustive right now.

Ok! Yes, seems you are right. I removed that commit.

Copy link
Member

@dlon dlon left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@faern faern changed the title Mark lots of enums #[non exhaustive] Mark lots of enums #[non_exhaustive] Apr 16, 2024
@faern faern merged commit a87e63c into main Apr 16, 2024
11 checks passed
@faern faern deleted the mark-enums-non_exhaustive branch April 16, 2024 07:37
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