Revert k6/http.head to not taking body as second argument #2402
Conversation
This has been the case forever until v0.36.0.
|
Ah, I probably messed it up with #2226 😞 Can you also add a test so we don't have the same regression again? Also, I wonder if we shouldn't add a "known bugs" sections to the release notes and edit the v0.36.0 ones? This will be fixed in v0.37.0 in a couple of weeks, so probably no need to make a v0.36.1 point release, but it might still be useful to have it until then |
mstoykov
force-pushed
the
fixHttpHead
branch
from
7ee2e41
to
d8ab553
Compare
|
@mstoykov, can you add something like this to the bottom of
We can edit the https://github.com/grafana/k6/releases/tag/v0.36.0 manually when we agree on the message |
js/modules/k6/http/http.go
Outdated
| @@ -92,7 +92,10 @@ func (r *RootModule) NewModuleInstance(vu modules.VU) modules.Instance { | |||
| args = append([]goja.Value{goja.Undefined()}, args...) // sigh | |||
| return mi.defaultClient.Request(http.MethodGet, url, args...) | |||
| }) | |||
| mustExport("head", mi.defaultClient.getMethodClosure(http.MethodHead)) | |||
| mustExport("head", func(url goja.Value, args ...goja.Value) (*Response, error) { | |||
| args = append([]goja.Value{goja.Undefined()}, args...) // sigh | |||
There was a problem hiding this comment.
| args = append([]goja.Value{goja.Undefined()}, args...) // sigh | |
| // It avoid creating dedicated logic reusing the common Request method | |
| // setting the body of the request to null. | |
| args = append([]goja.Value{goja.Undefined()}, args...) |
Sorry for seeing it just now, but if we want a comment there then I think we should use the opportunity for explaining why we are doing it.
There was a problem hiding this comment.
hmm something like this will be better, I think:
| args = append([]goja.Value{goja.Undefined()}, args...) // sigh | |
| // http.head(url, params) doesn't have a body argument, so we add undefined | |
| // as the third argument to http.request(method, url, body, params) | |
| args = append([]goja.Value{goja.Undefined()}, args...) // sigh |
There was a problem hiding this comment.
+1 to this. I would rephrase it:
It avoids having to create some dedicated logic to reuse the common Request method
|
+1 to adding a "Known bugs" indeed. Moreover, as @codebien mentioned, we could probably release a "patch" release too? |
This has been the case forever until v0.36.0.