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

Rename action tags from camel case to path format #837

Closed
andrevmatos opened this issue Jan 7, 2020 · 2 comments · Fixed by #1623
Closed

Rename action tags from camel case to path format #837

andrevmatos opened this issue Jan 7, 2020 · 2 comments · Fixed by #1623

Comments

@andrevmatos
Copy link
Contributor

Description

On #806 , with async actions, we introduced a new naming schema for actions, which looks like paths in a domain driven structure
example:
Before:

  • channelOpen (request)
  • channelOpened (success)
  • channelOpenFailed (failure)

After:

  • channel/open/request
  • channel/open/success
  • channel/open/failure

We want to be consistent with this tagging schema for the rest of the actions as well.
Examples:

  • block/new
  • token/monitor
  • iou/clear

These literals should also be used only on the definition of the actions, and anywhere else, action.type should be used instead of the string.

Acceptance criteria

Tasks

  • [ ]
@kelsos
Copy link
Contributor

kelsos commented Jun 3, 2020

Would it make sense to provide constants for the actions exposes via the public API as event types? while modifying the names I realized that this actually requires a couple of changes to events that the user of the SDK might listen for.

This means that we are actually exposing internal implementation to the users. Constants as an alternative, only for the public exposed events would mean that in case of rename the user of the SDK would not need to do much, unlike now that they need to go and rename any type filtering.

@andrevmatos
Copy link
Contributor Author

I'd say not, the action's type is already a literal, so it IS the constant. At most, to avoid the user needing to import the actions as it's too internal, we could aggregate the public events tags in some public const object or enum, to be exported/imported by the users and used where events are needed, but I'd say we still need to figure out how the events will look like before doing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants