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

Fix sobek values for ElementHandle #4567

Merged
merged 3 commits into from
Feb 19, 2025

Conversation

inancgumus
Copy link
Member

@inancgumus inancgumus commented Feb 19, 2025

What?

Fixes Sobek value usage for ElementHandle.

Why?

Fixing potential race conditions.

Checklist

  • I have performed a self-review of my code.
  • I have commented on my code, particularly in hard-to-understand areas.
  • I have added tests for my changes.
  • I have run linter and tests locally (make check) and all pass.

Checklist: Documentation (only for k6 maintainers and if relevant)

Please do not merge this PR until the following items are filled out.

  • I have added the correct milestone and labels to the PR.
  • I have updated the k6-documentation: grafana/k6-docs#PR-NUMBER
  • I have updated the TypeScript definitions: grafana/k6-DefinitelyTyped#PR-NUMBER
  • I have updated the release notes: link.

Related PR(s)/Issue(s)

@inancgumus inancgumus added this to the v1.0.0-rc1 milestone Feb 19, 2025
@inancgumus inancgumus self-assigned this Feb 19, 2025
This is to use it from the mapping layer.
@inancgumus inancgumus force-pushed the fix/sobek-values-for-element-handle branch from df9ea97 to a67a669 Compare February 19, 2025 16:31
@inancgumus inancgumus force-pushed the fix/sobek-values-for-element-handle branch from e9fdaa6 to e848b77 Compare February 19, 2025 17:15
@inancgumus inancgumus marked this pull request as ready for review February 19, 2025 17:15
@inancgumus inancgumus requested a review from a team as a code owner February 19, 2025 17:15
@inancgumus inancgumus requested review from mstoykov and olegbespalov and removed request for a team February 19, 2025 17:15
Copy link
Contributor

@mstoykov mstoykov left a comment

Choose a reason for hiding this comment

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

LGTM!

I wonder if Files needs to be a type at all or should Parse just return []File, error instead. But I guess this way it looks more like the others.

@inancgumus
Copy link
Member Author

inancgumus commented Feb 19, 2025

I wonder if Files needs to be a type at all or should Parse just return []File, error instead. But I guess this way it looks more like the others.

Totally agree. No need for it to be a type. I just focused on the current issue, but it definitely makes sense to remove that type.

@inancgumus inancgumus merged commit 1e9994c into master Feb 19, 2025
28 checks passed
@inancgumus inancgumus deleted the fix/sobek-values-for-element-handle branch February 19, 2025 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants