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

Remove x/nft enum declaration #252

Closed
husio opened this issue Jan 2, 2019 · 0 comments · Fixed by #261
Closed

Remove x/nft enum declaration #252

husio opened this issue Jan 2, 2019 · 0 comments · Fixed by #261
Assignees

Comments

@husio
Copy link
Contributor

husio commented Jan 2, 2019

In x/nft we are using enum as a const. There is no need for this enum declaration as it is not part of any protobuf message. We can declare those values as const strings in Go code instead.
Such change will reduce the amount of code, remove confusing protobuf enum declaration and keep action names constant to avoid typos.

@husio husio added the cleanup label Jan 2, 2019
@husio husio self-assigned this Jan 7, 2019
husio added a commit that referenced this issue Jan 7, 2019
Protobuf generated `Action` enum is replaced by a custom `Action` string
type.
This refactoring does not introduce and external API changes. Both JSON
and binary format is kept.

resolve #252
@alpe alpe closed this as completed in #261 Jan 11, 2019
alpe pushed a commit that referenced this issue Jan 11, 2019
* refactor `x/nft.Action` type

Protobuf generated `Action` enum is replaced by a custom `Action` string
type.
This refactoring does not introduce and external API changes. Both JSON
and binary format is kept.

resolve #252

* use regexp to validate nft action string

We do not know the full list of possible nft actions, so when validating
an action name, we can only ensure that if follows certain naming
scheme.

* Redo nft actions registration.

Instead of using regexp to validate an action, provide a global action
index where all actions must be registered before using.

* statically register default nft actions

Instead of using init function, statically register all actions that
support for is implemented by the nft extension.

* add actions to app.go

* add default actions array

* fix nft tests
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 a pull request may close this issue.

1 participant