-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Merge browser in k6 #4056
Merged
Merged
Merge browser in k6 #4056
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This function will call the handlers one by one, allowing each handler to check the url matches and return the name. Each handler will overwrite the name if multiple handlers match with the given url. This function needs to wait for the handler to complete before proceeding to the next handler.
Co-authored-by: İnanç Gümüş <inanc.gumus@grafana.com>
This makes working with an atomic int more maintainable.
Instead of working with goroutines and channels to propagate the name back from GroupURLTag, a name field is updated in ExportedMetric. The call to the taskqueue is synchronised so when the handler is called we can safely wait for it to complete instead of relying on channels and contexts to wait for it to complete.
This better represents what is happening. When a match is found, the metric is tagged. It is not grouping anything, the grouping is a wanted side effect.
We're now only creating a single MetricEvent for all handlers.
…ort. Fallback to defaults in case of visualViewport is nil
See #4107 for contextcheck Revive also doesn't make as much sense for code that is not supposed to be used directly by third parties, especially given the size of the API.
mstoykov
force-pushed
the
mergeBrowserInK6
branch
from
December 12, 2024 13:06
2b587dd
to
c4b2ca8
Compare
ankur22
approved these changes
Dec 12, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
oleiade
approved these changes
Dec 13, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! 🚀 🚀 🚀
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Move xk6-browser inside k6 codebase
Why?
Browser is no longer experimental and it makes more sense to have a single repository
List of commands I've ran:
git subtree add -P js/modules/k6/browser git@github.com:grafana/xk6-browser.git main
- requires git-subtree - in practice can be done with a bunch of additional commands, just this is faser and easiergit rm -r CODE_OF_CONDUCT.md CODEOWNERS CONTRIBUTING.md docker-compose.yaml Dockerfile go.mod go.sum LICENSE Makefile README.md register.go SECURITY.md SUPPORT.md sync_register.go TROUBLESHOOTING.md release\ notes/ assets .golangci.yml .gitignore .github/
git commit -m "browser: cleanup unnsecessary files" -n
find . -iname '*.go' | xargs sed -i -r 's|github.com/grafana/xk6-browser|go.k6.io/k6/js/modules/k6/browser|g'
git commit -m "Move to using the in k6 browser code"
git mv js/modules/k6/browser/examples/ examples/browser && git rm examples/browser.js examples/experimental/browser.js && git commit -m "Move browser examples and clean old ones" -a
cd examples/browser/ && sed -i -r 's!/x/browser/async!/browser!g' * && git commit -m "Fix imports in browser examples"
cd ../../js/modules/k6/browser && git rm -r sync-examples/ && git commit -m "browser: remove sync examples"
TODO:
Checklist
make lint
) and all checks pass.make tests
) and all tests pass.Related PR(s)/Issue(s)