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

Split mappings to prepare for async migration #1298

Merged
merged 14 commits into from
May 3, 2024
Merged

Conversation

inancgumus
Copy link
Member

What?

Splitting the component mappings from a single file into separate files.

Why?

While working on the sync-to-async migration (#428), this refactoring has the following benefits:

  • Less merging problems.
  • Focused mappings for each component (ElementHandle, Page, etc.) (less merging problems).
  • No need to deal with merging main-async (a possible branch name) into main, which might bring other merge conflicts if we can't deliver the work in a single cycle.

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)

#428

@inancgumus inancgumus added refactor async supports async (promises) labels May 3, 2024
@inancgumus inancgumus self-assigned this May 3, 2024
@inancgumus inancgumus marked this pull request as ready for review May 3, 2024 11:44
@inancgumus inancgumus requested a review from ankur22 May 3, 2024 11:44
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 mentioned this pull request May 3, 2024
@inancgumus inancgumus merged commit f99418b into main May 3, 2024
17 checks passed
@inancgumus inancgumus deleted the refactor/split-mappings branch May 3, 2024 11:59
@inancgumus inancgumus changed the title Split mappings Split mappings to prepare for async migration May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
async supports async (promises) refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants