-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Add user actions support #39141
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
Add user actions support #39141
Conversation
7f78431
to
71ea6fe
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.
LGTM, appreciate the added consistency on the constants.
The one thing I initially questioned was if an empty action was possible, but I see in the SDK that the json annotations for action include omitempty
and this matches with the lack of output changes in existing tests.
Will mark ready for merge once CI is satisfied.
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.
LGTM 👍
One small nit: I’m not a huge fan of prefixing all constants with faro inside the same package. Since they're unexported, we avoid stutter, so it’s mostly a stylistic point. Not a blocker — feel free to merge as-is.
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Adding support for user actions feature faro web SDK recently introduced grafana/faro-web-sdk#1033 (alpha version). Plus small fixes like: - add more tests - use constants where they should be used <!--Describe what testing was performed and which tests were added.--> #### Testing Added unit tests --------- Co-authored-by: Sam DeHaan <[email protected]>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Adding support for user actions feature faro web SDK recently introduced grafana/faro-web-sdk#1033 (alpha version). Plus small fixes like: - add more tests - use constants where they should be used <!--Describe what testing was performed and which tests were added.--> #### Testing Added unit tests --------- Co-authored-by: Sam DeHaan <[email protected]>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Adding support for user actions feature faro web SDK recently introduced grafana/faro-web-sdk#1033 (alpha version). Plus small fixes like: - add more tests - use constants where they should be used <!--Describe what testing was performed and which tests were added.--> #### Testing Added unit tests --------- Co-authored-by: Sam DeHaan <[email protected]>
Description
Adding support for user actions feature faro web SDK recently introduced grafana/faro-web-sdk#1033 (alpha version).
Plus small fixes like:
Testing
Added unit tests