-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
WIP: (Low priority) Use enum struct
and drop enum variant prefixes
#3453
Conversation
enum strut
and drop prefixesenum struct
and drop prefixes
Hmm. CI timed out. |
It did not really time out. You are running in a bunch of these errors on Darwin:
|
Oh oops I read the wrong one. Thanks! |
This does a few enums; the rest will be gotten in subsequent commits.
OK! this all up date and ready to go. |
enum struct
and drop prefixesenum struct
and drop enum variant prefixes
@edolstra This PR in particular especially clutters up the other more interesting PRs that contain it. It would be great if we could merge it soon. My offer still stands to get rid of any conflicts in the flakes after merging :). |
I'm not really in favor of this. It's a huge amount of code churn for no real advantage (as far as I can see). I mean, what's the benefit to writing |
My apologies for not expounding "better practice in C++", but it appears according to https://en.cppreference.com/w/cpp/language/enum that these "scoped enumerations" have fewer implicit conversions than their unscoped equivalents. I would think that's some real value as we don't want numbers sneaking through. In addition, I still find the code with change easier to read:
I'd hope one of the main benefits of not keeping a stable API in Nix anyways is that there is little-to-no marginal cost in doing slight cleanups like there. Each one is small, but they'll compound over time. And with my promise to go through any unmerged branch you like to resolve the conflicts, I hope the churn wouldn't impact you at all. But if you still don't like this, we can close it and I'll go remove it from the other PRs it's included in. That will be tedious, but is self-inflicted on my part, so I'm not asking that it affect your decision. |
enum struct
and drop enum variant prefixesenum struct
and drop enum variant prefixes
enum struct
and drop enum variant prefixesenum struct
and drop enum variant prefixes
If we have fix all the lower hanging fruit such that churn like this is deemed with it, I assume it will be easier to just |
This does a few enums; the rest will be gotten in subsequent commits.
Thanks @jhartzell42 for teaching me about this. It's better practice in C++, and, as a bonus, it's closer to Rust shold we wish to rewrite more :O.
@edolstra I know this is a lot of churn. I'll be happy to take care of the merge into the flakes branch (or other branches you deem important) if you like, to lessen the cost of this on other devs.