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.

Sign up for GitHub

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

Jump to bottom

Refactor page.on mapping names #1478

Merged
merged 17 commits into from
Oct 11, 2024
Merged

Refactor page.on mapping names #1478

merged 17 commits into from
Oct 11, 2024

Conversation

inancgumus
Copy link
Member

What?

Improves namings in page.on mapping.

Why?

To improve clarity and provide the first step for the page.on generalization work.

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

@inancgumus
Move pageOn mapping to a function
80e0a92 
The page on mapping is large and hard to work with while making the
changes. Putting it into a new function is subjetively more readable and
makes it easier to work with the function.
@inancgumus
Remove unnecessary vu.VU calls from mapPageOn
7fa8233
@inancgumus
Move VU runtime to variable in mapPageOn
adeb3a6
@inancgumus
Rename runInTaskQueue to mapHandler
ea4af1f 
No need to specify the implementation details that can change. Our main
goal here is to map the page on event handler to sobek.
@inancgumus
Inline console API mapping closure
49e25b3 
We're uniforming the events.
@inancgumus
Extract page on console
ef41349
@inancgumus
Extract page on metric
00ae271
@inancgumus
Refactor mapConsoleMessage to PageOnEvent
a800d9d 
This is to uniform all the page on event mappers.
@inancgumus
Use cm variable in mapConsoleMessage
5c17e01
@inancgumus
Refactor mapConsoleMessage out of sobek
0cd31a1 
No need to wrap the return values into sobek ones.
@inancgumus
Refactor mapMetricevent to PageOnEvent
9e46294
@inancgumus
Remove excessive VU call mapMetricEvent
0e8566b
@inancgumus
Use em variable in mapMetricEvent
755208e
@inancgumus
Extract err from mapMetricEvent call
586bdef
@inancgumus
Rename channel c to done
f1a589f
@inancgumus
Rename event to eventName
c95ba38
@inancgumus
Rename mapPageOn handler to handleEvent
110effe
@inancgumus inancgumus self-assigned this Oct 10, 2024
@inancgumus inancgumus marked this pull request as ready for review October 10, 2024 19:42
@inancgumus inancgumus mentioned this pull request Oct 10, 2024
Closed
@inancgumus
Copy link
Member Author

inancgumus commented Oct 10, 2024 •
edited
Loading

No idea why the linter fails like this (it does not do so locally):

  Error: vu.Runtime undefined (type moduleVU has no field or method Runtime) (typecheck)
  Error: vu.Runtime undefined (type moduleVU has no field or method Runtime) (typecheck)

This might be a linter bug.

ankur22
ankur22 approved these changes Oct 11, 2024
View reviewed changes
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

browser/metric_event_mapping.go Show resolved Hide resolved
@inancgumus inancgumus merged commit 91ff150 into main Oct 11, 2024
22 of 23 checks passed
@inancgumus inancgumus deleted the refactor/page-on-mapping-namings branch October 11, 2024 13:16
@BrewTestBot BrewTestBot mentioned this pull request Nov 11, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ankur22 ankur22 ankur22 approved these changes

Assignees

@inancgumus inancgumus

Labels
refactor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@inancgumus @ankur22