-
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
Release/v0.36.0 #2340
Release/v0.36.0 #2340
Conversation
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 spot some but not all, generally, we should exec s/K6/k6/
for branding consistency.
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've noticed some simple improvements but also added a couple of more complex suggestions: please see.
Also, 27 closed issues! 😲 They accumulated probably (like abort test) but still, that's almost twice more than prev time.
b268bc9
to
6b95450
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, @oleiade, for the update! I think with link to the docs, new interfaces in Extensions
section are just right.
Just a few re-phrasing suggestions from my side 🙂
e01c0c0
to
bdf8030
Compare
9b064ec
to
f261286
Compare
LGTM overall, these are the only nitpicks I have |
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.
Small things
0225690
to
b6ed53b
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.
@oleiade LGTM, thanks for doing it 👏
We should link grafana/k6-docs#556 and grafana/k6-docs#559 when they will be merged
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 besides the minor nitpicks
This PR can now be considered ready to merge. To proceed, we should ensure to |
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 👍
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 apart from the ... more accurate wording of why 'optional chaining' isn't usable.
In this case I mean that if you just have var something = {"something": {}}; console.log(something?.something?.else?.third);
will work as babel is not used at all. But needing babel, usually for import/export, means that you can't use it.
e48d0ec
to
b47ad1b
Compare
This PR adds release notes for the upcoming 0.36.0 version of K6.
To compile them, I went through the commit history from 0.35.0 up to the current state of master, and looked at/experimented with most PRs that went through during this period. I might have missed things, overlooked overs, left changes so small that I didn't find them too relevant for release notes.
Please let me know if you think I've forgotten anything, explained something wrong, or didn't follow the existing template close enough. This is my first time, and I'm looking to get as much feedback as possible 🐻
Not being too familiar with SourceMaps myself, I could definitely use your help @mstoykov when it comes to describing and demonstrating the feature. Could you support me with that? 😸