Skip to content

Conversation

@jbudz
Copy link
Member

@jbudz jbudz commented Sep 18, 2017

This reverts #13694 and reopens #13687. Ideally we could let this sit, but large chunks of our documentation make use of GET requests with bodies. In these cases the body would be ignored.

/cc @chrisronline

Copy link
Member

@tylersmalley tylersmalley left a comment

Choose a reason for hiding this comment

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

One small change.

var wrappedDfd = $.Deferred();

console.log("Calling " + path);
var isGETRequest = /get/i.test(method)
Copy link
Member

Choose a reason for hiding this comment

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

Can we ensure the entire string is get and camelcase the variable name?

var isGetRequest = /^get$/i.test(method)

@tylersmalley
Copy link
Member

Should this go back to 6.0?

@jbudz
Copy link
Member Author

jbudz commented Sep 20, 2017

I'm going to revert the last commit to get the master and 6.1 changes out of the way. And then I'll do the get request changes back to 5.6 in a new PR

@tylersmalley
Copy link
Member

What's the reason for reverting the case insensitivity?

@jbudz
Copy link
Member Author

jbudz commented Sep 21, 2017

Only to make the backport to 6.0 and 5.6 clean - the PR this is reverting only went to 6.1 and 7.0. I'll open a followup after.

var wrappedDfd = $.Deferred();

console.log("Calling " + path);
if (data && method == "GET") {
Copy link
Member

Choose a reason for hiding this comment

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

this needs to be case insensitive, quite often folks do get /something and then the body is ignored.

@Mpdreamz Mpdreamz dismissed their stale review September 21, 2017 13:56

Just spotted c749f8d#r139834769, would love to see this go in 6.0 and 5.6 though,

@jbudz
Copy link
Member Author

jbudz commented Sep 22, 2017

Yes, planning on taking it back to 5.6.

@tylersmalley
Copy link
Member

Ok, let's get this in and I have created a follow-up issue: #14280

@jbudz jbudz merged commit 2c430de into elastic:master Oct 5, 2017
jbudz added a commit that referenced this pull request Oct 5, 2017
* Revert "[console] Don't infer request method (#13694)"

This reverts commit 38b13c7.

* [console] Make request method check case insensitive

* Revert "[console] Make request method check case insensitive"

This reverts commit c749f8d.
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