-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Some enum #17435
base: main
Are you sure you want to change the base?
Some enum #17435
Conversation
That allows to remove a else -> and improve typing.
6b5acb0
to
33a3ae4
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.
Note: both of these classes are planned to be deleted.
Signal is fine. Good change
I don't like Caller
from an architecture POV, but I don't feel strongly here.
I do want to stop using implicit enum ordinals, and that's what I'll request changes on: that makes the order of the enums a source of bugs.
ADD_IMAGE, | ||
INSTANT_NOTE_EDITOR; | ||
companion object { | ||
fun Int.toCaller() = Caller.entries[this] |
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.
This is a recipe for disaster, keep the enum ints explicit
This now throws on an unexpected value
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.
If this were expected to survive, I'd prefer to see an acra report on unexpected value. Because that's unexpected, and I'd prefer to know this occurs. Because that means that we probably don't do what we expect to do when the collection is loaded
Improve typing
33a3ae4
to
0fc1957
Compare
Those commits add some enum for the sake of code clarity