Skip to content

Patch: analytics events 11#6303

Merged
zachmargolis merged 3 commits intoLG-5929-document-analytics-11from
margolis-patch-analytics-events-11
May 4, 2022
Merged

Patch: analytics events 11#6303
zachmargolis merged 3 commits intoLG-5929-document-analytics-11from
margolis-patch-analytics-events-11

Conversation

@zachmargolis
Copy link
Contributor

I have some additional review feedback for #6293 but it seemed easier to just make this PR instead of leave a bunch more comments

See commits for reasoning, the most important is d1961a1 because once the changes land on main they would break without it

- As of #6294, having it will cause build breakage since it's
  an unknown tag
@zachmargolis zachmargolis requested a review from gsa-manish May 4, 2022 15:39
Copy link
Contributor

@gsa-manish gsa-manish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx for taking care of removing event_name tags.

@zachmargolis zachmargolis merged commit cb1c686 into LG-5929-document-analytics-11 May 4, 2022
@zachmargolis zachmargolis deleted the margolis-patch-analytics-events-11 branch May 4, 2022 18:15
gsa-manish added a commit that referenced this pull request May 5, 2022
* LG-5929-document-analytics-11

* Patch: analytics events 11 (#6303)

* Remove @identity.idp.event_name

- As of #6294, having it will cause build breakage since it's
  an unknown tag

* Remove blank lines

* Add clearer comments for each event

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
peggles2 pushed a commit that referenced this pull request May 5, 2022
* LG-5929-document-analytics-11

* Patch: analytics events 11 (#6303)

* Remove @identity.idp.event_name

- As of #6294, having it will cause build breakage since it's
  an unknown tag

* Remove blank lines

* Add clearer comments for each event

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
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 this pull request may close these issues.

2 participants