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

chore(storage): smallish refactoring series, part 1 #383

Merged
merged 2 commits into from
Feb 2, 2020

Conversation

fredbi
Copy link
Contributor

@fredbi fredbi commented Feb 1, 2020

Removed aliased type to bool.

Go aliased types (i.e. type A = B) are a syntactic sugar only.
This is different from type redefinitions (i.e. type A B), which create a new type for the compiler.

The original intent was to construct some kind of enumerated type, for a... boolean.

I chose to remove all this and just go back to plain bool.

Signed-off-by: Frederic BIDON [email protected]

Removed aliased type to bool.

Go aliased types (i.e. type A = B) are a syntactic sugar only.
This is different from type redefinitions (i.e. type A B), which create a new type for the compiler.

The original intent was to construct some kind of enumerated type, for a... boolean.

I chose to remove all this and just go back to plain bool.

* contributes #374

Signed-off-by: Frederic BIDON <[email protected]>
Signed-off-by: Frederic BIDON <[email protected]>
@kerneltime
Copy link
Contributor

The goal was to force use of the named values vs. true and false which require jumping around in code to know what it means.. I am ok with a builder pattern to executes similar to the google sdk

@fredbi
Copy link
Contributor Author

fredbi commented Feb 1, 2020

I understood the goal, and enforcement is not achieved with aliased types.
If you want an enum (well, for true, false, ...) you need to redefine it this way type KeyX bool.

I feel that the choice of wording in the constants was at best misleading, and ended up with something less readable than the plain overwrite bool.

@kerneltime kerneltime merged commit 5ed7e6d into master Feb 2, 2020
@fredbi fredbi deleted the smallish-refact branch February 2, 2020 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants