-
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
Adds xk6-browser v1.0.2 release notes #3260
Conversation
Codecov Report
@@ Coverage Diff @@
## release-notes-v046 #3260 +/- ##
=====================================================
Coverage ? 73.22%
=====================================================
Files ? 259
Lines ? 19899
Branches ? 0
=====================================================
Hits ? 14571
Misses ? 4404
Partials ? 924
Flags with carried forward coverage won't be shown. Click here to find out more. |
c2a8c21
to
be5dbd5
Compare
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.
Thanks for this! This is great 👍
I left some pretty minor suggestions to ponder over.
e41296c
to
86e3f0b
Compare
release notes/v0.46.0.md
Outdated
@@ -8,6 +8,19 @@ k6 `v0.46` is here 🎉! This release includes: | |||
|
|||
- `#pr`, `<small_break_1>` | |||
- `#pr`, `<small_break_2>` | |||
- [browser#868](https://github.com/grafana/xk6-browser/pull/868) Removes support for browser `devtools` option. |
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.
I wonder if this shouldn't be in the browser section below. And here just to be "Browser has a number of breaking changes due to change in vision on how it will be used, full list -> link"
Currently, The whole first page of the release notes is breaking changes ... and they are only in browser.
Anyone not interested in that will just gloss over them, potentially missing something (if we have other ones).
Additionally reading this list will not really tell users they likely need to rewrite big parts of their scripts(or at least how to start browsers) - this happens in the other section.
cc @grafana/k6-core
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.
Which browser section? You mean the "Automatic Browser Lifecycle Handling" point in "New Features" section? Or you mean adding a dedicated browser section at least for this release?
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.
Personally I think providing the full list as it is now it's ok, but maybe we could add an H3 header just before the browser changes inside the "Breaking changes" section with an introduction to the changes and links to the "Automatic Browser Lifecycle Handling" point, which summarizes most of the important changes, similar to what you said I believe.
Our idea is to write a migration guide that will assist users in this transition, but unfortunately this is not yet ready so we can link it directly from the release notes.
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.
I would suggest having the following structure:
Breaking changes
Automatic Browser Lifecycle Handling
A small introduction (as you already added in the current feature section)
- the list of breaking changes
It sounds a bit unfriendly to get directly the list of breaking changes without any introduction that provides context about so many breaking changes. For the same reason, it makes sense to me to have them all together in the same section as I shared here #3260 (comment).
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.
We can add a link to the migration guide here as well. More info: #3260 (comment)
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.
See a proposal for reorganization in 9349278.
Resolves #3260 (comment) Co-authored-by: Ankur <[email protected]>
release notes/v0.46.0.md
Outdated
@@ -89,6 +143,22 @@ _Format as `<number> <present_verb> <object>. <credit>`_: | |||
- [#3136](https://github.com/grafana/k6/pull/3136) Updates k6 Go dependencies. | |||
- [#3131](https://github.com/grafana/k6/pull/3131) Updates our Pull Request template to be more structured and include a checklist for contributors. | |||
- [#3177](https://github.com/grafana/k6/pull/3177) Updates the version of golangci-lint we use to the latest version. | |||
- [browser#849](https://github.com/grafana/xk6-browser/pull/849) Updates `nil` session log warn to debug level in page attach. |
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.
I'd recommend trying to squash things whenever this is possible. And also, try to see for the record first of all from the k6's user perspective (I mean, how valuable is the record for the user).
Like instead of:
- Refactored A
- Refactored B
- Added test C
It could be easily:
- Internal refactorings and test improvements (#A, #B, #C)
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.
Personally I would remove many of these internal changes that do not impact the end user. How does this diff look to you?
cc: @ankur22 @inancgumus
diff
diff --git a/release notes/v0.46.0.md b/release notes/v0.46.0.md
index 1f2528a9..d90d7cce 100644
--- a/release notes/v0.46.0.md
+++ b/release notes/v0.46.0.md
@@ -143,22 +143,12 @@ _Format as `<number> <present_verb> <object>. <credit>`_:
- [#3136](https://github.com/grafana/k6/pull/3136) Updates k6 Go dependencies.
- [#3131](https://github.com/grafana/k6/pull/3131) Updates our Pull Request template to be more structured and include a checklist for contributors.
- [#3177](https://github.com/grafana/k6/pull/3177) Updates the version of golangci-lint we use to the latest version.
-- [browser#849](https://github.com/grafana/xk6-browser/pull/849) Updates `nil` session log warn to debug level in page attach.
-- [browser#889](https://github.com/grafana/xk6-browser/pull/889) Refactors `isRemoteBrowser` check.
-- [browser#889](https://github.com/grafana/xk6-browser/pull/899) Refactors pid registry.
-- [browser#902](https://github.com/grafana/xk6-browser/pull/902) Refactors registry back into browser package.
-- [browser#904](https://github.com/grafana/xk6-browser/pull/904) Fixes a race condition in Web Vitals unit test.
-- [browser#906](https://github.com/grafana/xk6-browser/pull/906) Parses the scenarios JSON definition from the `K6_INSTANCE_SCENARIOS` environment variable.
- [browser#918](https://github.com/grafana/xk6-browser/pull/918) Replaces the usage of `os.LookupEnv` for k6 `LookupEnv` and abstracts environment variables lookup.
-- [browser#935](https://github.com/grafana/xk6-browser/pull/935) Refactors the integration test browser implementation.
-- [browser#936](https://github.com/grafana/xk6-browser/pull/936) Removes the `BaseEventEmitter` from browser.
-- [browser#959](https://github.com/grafana/xk6-browser/pull/959) Fixes `TestFrameNoPanicNavigateAndClickOnPageWithIFrames` test.
- [browser#972](https://github.com/grafana/xk6-browser/pull/972) Fixes log level from `warn` to `debug` when `fetchBody` fails due to `context` being canceled on iteration end.
- [browser#977](https://github.com/grafana/xk6-browser/pull/977) Removes goroutine ID field from logger.
- [browser#978](https://github.com/grafana/xk6-browser/pull/978) Adds browser extension source log field.
- [browser#983](https://github.com/grafana/xk6-browser/pull/983) Avoids `onRequestPaused` error log when `context` is canceled.
- [browser#984](https://github.com/grafana/xk6-browser/pull/984) Ensures CDP requests message ID are unique at the connection scope.
-- [browser#992](https://github.com/grafana/xk6-browser/pull/992) Fixes go mod prometheus dependency issue.
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.
I would prefer to keep the internal changes but consolidate them as olegbespalov suggested.
As a side note, would the log level changes from warn to debug be classed as improvements since the logs will be less noisy which will impact a user when they're debugging their test?
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.
I still think there are changes in the list that are irrelevant for end users, and grouping them just makes the other elements that fall in the same category but are more important to lose "weight/relevance" in the list.
Trying to follow your suggestions, how does this look?
diff
diff --git a/release notes/v0.46.0.md b/release notes/v0.46.0.md
index 1f2528a9..72438375 100644
--- a/release notes/v0.46.0.md
+++ b/release notes/v0.46.0.md
@@ -143,22 +143,12 @@ _Format as `<number> <present_verb> <object>. <credit>`_:
- [#3136](https://github.com/grafana/k6/pull/3136) Updates k6 Go dependencies.
- [#3131](https://github.com/grafana/k6/pull/3131) Updates our Pull Request template to be more structured and include a checklist for contributors.
- [#3177](https://github.com/grafana/k6/pull/3177) Updates the version of golangci-lint we use to the latest version.
-- [browser#849](https://github.com/grafana/xk6-browser/pull/849) Updates `nil` session log warn to debug level in page attach.
-- [browser#889](https://github.com/grafana/xk6-browser/pull/889) Refactors `isRemoteBrowser` check.
-- [browser#889](https://github.com/grafana/xk6-browser/pull/899) Refactors pid registry.
-- [browser#902](https://github.com/grafana/xk6-browser/pull/902) Refactors registry back into browser package.
-- [browser#904](https://github.com/grafana/xk6-browser/pull/904) Fixes a race condition in Web Vitals unit test.
+- [browser#849](https://github.com/grafana/xk6-browser/pull/849), [browser#972](https://github.com/grafana/xk6-browser/pull/972), [browser#977](https://github.com/grafana/xk6-browser/pull/977), [browser#978](https://github.com/grafana/xk6-browser/pull/978), [browser#983](https://github.com/grafana/xk6-browser/pull/983) Log fixes and improvements.
+- [browser#889](https://github.com/grafana/xk6-browser/pull/889), [browser#889](https://github.com/grafana/xk6-browser/pull/899), [browser#902](https://github.com/grafana/xk6-browser/pull/902), [browser#935](https://github.com/grafana/xk6-browser/pull/935), [browser#936](https://github.com/grafana/xk6-browser/pull/936) Internal refactors.
+- [browser#904](https://github.com/grafana/xk6-browser/pull/904), [browser#959](https://github.com/grafana/xk6-browser/pull/959) Test fixes.
- [browser#906](https://github.com/grafana/xk6-browser/pull/906) Parses the scenarios JSON definition from the `K6_INSTANCE_SCENARIOS` environment variable.
- [browser#918](https://github.com/grafana/xk6-browser/pull/918) Replaces the usage of `os.LookupEnv` for k6 `LookupEnv` and abstracts environment variables lookup.
-- [browser#935](https://github.com/grafana/xk6-browser/pull/935) Refactors the integration test browser implementation.
-- [browser#936](https://github.com/grafana/xk6-browser/pull/936) Removes the `BaseEventEmitter` from browser.
-- [browser#959](https://github.com/grafana/xk6-browser/pull/959) Fixes `TestFrameNoPanicNavigateAndClickOnPageWithIFrames` test.
-- [browser#972](https://github.com/grafana/xk6-browser/pull/972) Fixes log level from `warn` to `debug` when `fetchBody` fails due to `context` being canceled on iteration end.
-- [browser#977](https://github.com/grafana/xk6-browser/pull/977) Removes goroutine ID field from logger.
-- [browser#978](https://github.com/grafana/xk6-browser/pull/978) Adds browser extension source log field.
-- [browser#983](https://github.com/grafana/xk6-browser/pull/983) Avoids `onRequestPaused` error log when `context` is canceled.
- [browser#984](https://github.com/grafana/xk6-browser/pull/984) Ensures CDP requests message ID are unique at the connection scope.
-- [browser#992](https://github.com/grafana/xk6-browser/pull/992) Fixes go mod prometheus dependency issue.
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.
Consolidation makes more sense 👍
grouping them just makes the other elements that fall in the same category but are more important to lose "weight/relevance" in the list.
+1
would the log level changes from warn to debug be classed as improvements since the logs will be less noisy which will impact a user when they're debugging their test?
I think any UX change better be on the list as its own, and logs are one of them. It might be important for some users.
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.
Coming back to this discussion, can you mention which one of the two diffs/presented options do you prefer or provide specific suggestions please? Thank you!
Personally I would opt for the option mentioned in this comment.
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.
I'm happy with the second suggestion.
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.
Fixed in 4cfd22b.
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, @ka3de can you rebase for resolving the conflicts, please?
Co-authored-by: Ankur <[email protected]>
Resolves #3260 (comment) Co-authored-by: Ankur <[email protected]>
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.
Nice! I like it 🙂
I've left one comment to consider, but otherwise it looks good.
Refactor the list of breaking changes for xk6-browser v1.0.2 by providing more context explanation and organization. Also removes the "Automatic Browser Lifecycle Handling" section written under "New Features" and merges its content inside the browser breaking changes section.
9349278
to
434e175
Compare
Resolves #3260 (comment) Co-authored-by: Ankur <[email protected]>
* Add release notes v0.46.0 file * Add release notes template for version 0.46 * changelog: a record for the * Fill release notes templates for version 0.46 * Bump k6 version to version 0.46 * Update release notes/v0.46.0.md Co-authored-by: İnanç Gümüş <[email protected]> * Add loki headers changes * Add cloud traces to the release notes * Remove redundant new line * Drop 3227 from misc section * Update release notes/v0.46.0.md Co-authored-by: Ivan <[email protected]> * Update release notes/v0.46.0.md Co-authored-by: Ivan <[email protected]> * feat: gRPC connection's TLS * Apply PR reviews suggestions * Move miscelaneous items in dedicated sections * Fix the mention of the grpc fix * Cloud output v2 changelog (#3161) Changelog entry for the Cloud output v2 --------- Co-authored-by: Oleg Bespalov <[email protected]> * Document xk6-grpc's change * Adds xk6-browser v1.0.2 release notes (#3260) * Update release notes/v0.46.0.md Co-authored-by: Ivan <[email protected]> * Update release notes/v0.46.0.md Co-authored-by: Ivan <[email protected]> * Update release notes/v0.46.0.md Co-authored-by: Ivan <[email protected]> * Update release notes/v0.46.0.md Co-authored-by: Ivan <[email protected]> * Update release notes/v0.46.0.md Co-authored-by: Heitor Tashiro Sergent <[email protected]> * Apply suggestions from code review Co-authored-by: Heitor Tashiro Sergent <[email protected]> * Revert consts.go version bump * Integrate oleg's review * Apply suggestions from code review Co-authored-by: Heitor Tashiro Sergent <[email protected]> Co-authored-by: Ivan <[email protected]> * Apply Heitor's suggestions * fix grpc issue mention consistency * Update release notes/v0.46.0.md Co-authored-by: Heitor Tashiro Sergent <[email protected]> * Iterate on the release notes * Update release notes/v0.46.0.md Co-authored-by: Ivan <[email protected]> * Iterate on v0.46 release notes * Update release notes/v0.46.0.md Co-authored-by: Ivan <[email protected]> --------- Co-authored-by: Ivan <[email protected]> Co-authored-by: Oleg Bespalov <[email protected]> Co-authored-by: İnanç Gümüş <[email protected]> Co-authored-by: Mihail Stoykov <[email protected]> Co-authored-by: Lukasz Gut <[email protected]> Co-authored-by: ka3de <[email protected]> Co-authored-by: Heitor Tashiro Sergent <[email protected]>
What?
Adds the release notes corresponding to xk6-browser
v1.0.2
.