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.

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

Desobekify Browser #1389

Merged
merged 1 commit into from
Jul 1, 2024
Merged

Desobekify Browser #1389

merged 1 commit into from
Jul 1, 2024

Conversation

inancgumus
Copy link
Member

@inancgumus inancgumus commented Jun 21, 2024

What?

  • Moves BrowserContextOptions out of the business logic.
  • Sets default options in NewBrowserContext to prevent nil pointer issues in the rest of the module. Later, we can turn some of these pointer fields into value types to take advantage of their zero values.
  • Updates tests to work with the new structure.

Note

These changes should had to be made in the single commit to be atomic.

Why?

See #1269.

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

@inancgumus inancgumus self-assigned this Jun 21, 2024
@inancgumus inancgumus marked this pull request as ready for review June 21, 2024 11:48
Copy link

@allansson allansson left a comment

Choose a reason for hiding this comment

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

This looks a lot cleaner. Removes a lot of the clutter of the business logic. Nice job! 😍

common/browser.go Show resolved Hide resolved
- Moves BrowserContextOptions out of the business logic.
- Sets default options in NewBrowserContext to prevent nil pointer
  issues in the rest of the module. Later, we might think about turning
some of these pointer fields into value types to take advantage of their
zero values.
@inancgumus inancgumus merged commit c3f099a into main Jul 1, 2024
23 checks passed
@inancgumus inancgumus deleted the desobekify/browser branch July 1, 2024 10:29
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