Skip to content
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

Fix method chaining in sys.Http #6920

Closed

Conversation

bendmorris
Copy link
Contributor

Fixes #6919

Shoutout to HaxeFoundation/haxe-evolution#36 which would be a cleaner fix.

@ncannasse
Copy link
Member

I don't understand why we have method chaining here in the first place since we don't use this style at all in order parts of the Haxe standard library.

@bendmorris
Copy link
Contributor Author

It is convenient, and strictly more useful than returning nothing. There aren't a lot of places in the API that make sense as a fluent interface. Output could be another example (out.writeInt8(1).writeInt8(2).writeString("apple")) but would require a similar generic hack.

@mockey
Copy link
Contributor

mockey commented Apr 4, 2018

I don't understand why we have method chaining here in the first place since we don't use this style at all in order parts of the Haxe standard library.

That's right, it's rather unusual and surely debatable. But it used to work...

@Simn
Copy link
Member

Simn commented Apr 17, 2018

I don't understand why we have method chaining here in the first place since we don't use this style at all in order parts of the Haxe standard library.

I agree with that. The chaining probably creeped in at some point and then was just dragged along. I'd be fine with changing this for Haxe 4, especially if there are problems with it now anyway.

@mockey
Copy link
Contributor

mockey commented Apr 17, 2018

@Simn Hey, are you back for good?

@Simn Simn self-assigned this Apr 20, 2018
@Simn Simn added this to the Backlog milestone Apr 20, 2018
Simn added a commit to Simn/haxe that referenced this pull request Apr 26, 2018
@Simn
Copy link
Member

Simn commented Apr 26, 2018

#6980

@Simn Simn closed this Apr 26, 2018
@bendmorris bendmorris deleted the http-method-chaining branch April 26, 2018 18:00
Simn added a commit that referenced this pull request May 3, 2018
* [std] disable Http method chaining

see #6920

* keep old behavior if hx3compat is defined
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

current Http implementation: Chaining doesn't work as it used to
4 participants