Skip to content
This repository has been archived by the owner on Jan 30, 2025. It is now read-only.

Refactor to type safe page.on event names #1477

Merged
merged 4 commits into from
Oct 10, 2024

Conversation

inancgumus
Copy link
Member

What?

Add and use PageOnEventName.

Why?

For greater readability, clarity, and type safety at the compile time.

Checklist

  • I have performed a self-review of my code
  • I have added tests for my changes
  • I have commented on my code, particularly in hard-to-understand areas

Related PR(s)/Issue(s)

Updates: #1227

This will be used by all page on event names. With this, we won't
require whether a provided event is valid. The compiler will enforce it
for us.
This is safe to do so as we will no longer pass a rogue string to this
method. Only the page on event names can be passed to this function.

Unless, of course, somebody converts a string with an incorrect name to
PageOnEventName to call this function. This will easily be catched in
reviews, and nobody would do such a thing. We follow the Go idiom of
being pragmatic instead of being highly defensive. Also, this method is
used by the mapping layer, and there is already an event name check.
@inancgumus inancgumus self-assigned this Oct 10, 2024
@inancgumus inancgumus marked this pull request as ready for review October 10, 2024 15:46
@inancgumus inancgumus requested a review from ankur22 October 10, 2024 15:46
@inancgumus inancgumus force-pushed the refactor/type-safe-page-on-event-names branch from 7573397 to 1573c7b Compare October 10, 2024 15:46
Copy link
Collaborator

@ankur22 ankur22 left a comment

Choose a reason for hiding this comment

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

lgtm

@inancgumus inancgumus merged commit 16a650a into main Oct 10, 2024
23 checks passed
@inancgumus inancgumus deleted the refactor/type-safe-page-on-event-names branch October 10, 2024 16:34
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants