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

Desobekify BrowserContext.grantPermissions options parsing #1511

Merged
merged 2 commits into from
Nov 6, 2024
Merged

Desobekify BrowserContext.grantPermissions options parsing #1511

merged 2 commits into from
Nov 6, 2024

Conversation

inancgumus
Copy link
Member

@inancgumus inancgumus commented Nov 5, 2024 •
edited
Loading

What?

Moves BrowserContext.grantPermissions Sobek-option parsing into the mapping layer.

Why?

  • Turns GrantPermissionsOptions to a value type from a pointer, as this type does not need to be used to be mutated. Using value types is likely to be less racy and more performant.
  • Moves Sobek transformation to the mapping layer.

Also, see the reasons in grafana/k6#4219.

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)

@inancgumus inancgumus added refactor stability runtime stability improvements labels Nov 5, 2024
@inancgumus inancgumus self-assigned this Nov 5, 2024
@inancgumus inancgumus force-pushed the desobekify/browser-context-grant-permission-options-parsing branch from e2fb899 to 6bcbb03 Compare November 5, 2024 17:49
@inancgumus inancgumus mentioned this pull request Nov 5, 2024
Closed
@inancgumus inancgumus requested a review from ankur22 November 5, 2024 17:57
@inancgumus inancgumus marked this pull request as ready for review November 5, 2024 17:57
@inancgumus inancgumus force-pushed the desobekify/browser-context-grant-permission-options-parsing branch 2 times, most recently from df900b8 to 998bb01 Compare November 5, 2024 20:23
@inancgumus
Add exportTo helper
ad80bc7
@inancgumus
Refactor GrantPermissionOptions parsing to mapping
74e5505 
- Turns GrantPermissionsOptions to a value type from a pointer as this
  type is not need to be used to be mutated.
- Moves Sobek transformation to the mapping layer.
@inancgumus inancgumus force-pushed the desobekify/browser-context-grant-permission-options-parsing branch from 998bb01 to 74e5505 Compare November 5, 2024 20:59
ankur22
ankur22 approved these changes Nov 6, 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 🚀

examples/geo.js Outdated Show resolved Hide resolved
@inancgumus inancgumus merged commit ea41b9e into main Nov 6, 2024
21 of 23 checks passed
@inancgumus inancgumus deleted the desobekify/browser-context-grant-permission-options-parsing branch November 6, 2024 15:51
@BrewTestBot BrewTestBot mentioned this pull request Jan 6, 2025
Merged
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Reviewers

@ankur22 ankur22 ankur22 approved these changes

Assignees

@inancgumus inancgumus

Labels
refactor stability runtime stability improvements
Projects
None yet
Milestone
No milestone
Development

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