Add and enforce analytics naming convention

[aduth]

🛠 Summary of changes

Adds a new custom Rubocop linter to enforce a naming convention for analytics, expecting that the first argument to track_event should be a symbol with the same name as the method.

Good:

module AnalyticsEvents
  def my_method
    track_event(:my_method)
  end
end

Bad:

module AnalyticsEvents
  def my_method
    track_event('Some other name')
#               ^^^^^^^^^^^^^^^^^ IdentityIdp/AnalyticsEventNameLinter: Event name must match the method name, expected `:my_method`
  end
end

This was discussed in Engineering Huddle 2023-10-05 (see notes).

The proposal here would not retroactively update existing analytics, but would enforce against all new analytics events moving forward. This is done by adding inline Rubocop disabling for existing events.

📜 Testing Plan

bundle exec rubocop

[mitchellhenke]

👏🏼

[zachmargolis]

LGTM! 🚀

(Assuming the existing exceptions will be added before merging?)

[zachmargolis]

TIL the note syntax!

[aduth]

Pretty new feature I think!

https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax#alerts

[aduth]

(Assuming the existing exceptions will be added before merging?)

They're there, but the changes are extensive so GitHub's changes preview collapses the file.

[matthinz]

Question: Would track_event(__method__) be acceptable or is that too opaque? (For the record, I am 👍 with this PR as-is)

[aduth]

Question: Would track_event(__method__) be acceptable or is that too opaque?

Hmm, interesting idea! I don't personally mind it. It does come close to being "clever" programming, but I can't really imagine a scenario where __method__ would obscure discoverability in a way that having the full method name would be better.

I'd probably lean toward keeping this as-is, but I don't feel strongly at all; mostly just care about having it be consistent.

[soniaconnolly]

LGTM. Thanks for doing this!

I can imagine that in the future we might want to require the method name as the first part of the analytics name and allow additional words afterward, but we can always add that later if we find we want it.

[aduth]

I can imagine that in the future we might want to require the method name as the first part of the analytics name and allow additional words afterward, but we can always add that later if we find we want it.

I'd be curious what this would look like, since my instinct would be that if it should be included in the event name, it could / should also be reflected in the method name?

[aduth]

Thanks all for the feedback! As mentioned in Slack, I'll plan to merge this and we can see how it works out. I'll also try to search for open pull requests that might fall victim to this if merged to main without accounting for the new linter.