Skip to content

chore: use enum for NoteType#1594

Merged
Mirko-von-Leipzig merged 4 commits intonextfrom
santiagopittella-enum-note-type
Jan 29, 2026
Merged

chore: use enum for NoteType#1594
Mirko-von-Leipzig merged 4 commits intonextfrom
santiagopittella-enum-note-type

Conversation

@SantiagoPittella
Copy link
Collaborator

closes #624

@SantiagoPittella SantiagoPittella force-pushed the santiagopittella-enum-note-type branch from e077af2 to 97d46f5 Compare January 26, 2026 21:58
Copy link
Collaborator

@Mirko-von-Leipzig Mirko-von-Leipzig left a comment

Choose a reason for hiding this comment

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

Just the integer cast change to enum matching pls

.try_into()?;
let note_type = NoteType::try_from(u64::from(value.note_type))?;
let note_type = proto::note::NoteType::try_from(value.note_type)
.map_err(|_| ConversionError::EnumDiscriminantOutOfRange)?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this required? Seems like its mapping from E -> E?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This try_from is implemented by prost, is the casting from the i32 to an Enum variant and returns a custom error

Copy link
Collaborator

Choose a reason for hiding this comment

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

Man that's so weird. What's even the point of the enum if they never actually use it in the code..

Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you!

@Mirko-von-Leipzig Mirko-von-Leipzig merged commit 27b19fb into next Jan 29, 2026
7 checks passed
@Mirko-von-Leipzig Mirko-von-Leipzig deleted the santiagopittella-enum-note-type branch January 29, 2026 11:57
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.

Use enum for note_type field in protobuf representation of NoteMetadata

3 participants