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

Just a little api critic. #1917

Closed
heartforit opened this issue Nov 19, 2015 · 6 comments
Closed

Just a little api critic. #1917

heartforit opened this issue Nov 19, 2015 · 6 comments

Comments

@heartforit
Copy link

Hi guys,

i really like this library, you guys made an awesome job. I like to use this library.
But, please provide a alias for request.del => request.delete, because i was confused, because the method name mostly represents the http method and that surly does sense to me.

Best,
mrlove

@simov
Copy link
Member

simov commented Nov 19, 2015

I think that might be a good idea. Of course delete is a reserved keyword, but I'm not sure if that may cause any issues as a property name.

@heartforit
Copy link
Author

Yeah, it would be interesting to figure that out. I guess it should not be a big deal, but i don´t have time to invest more on it yet. I just like to give my two cents to it.. :) Clear api is a good basis for easy readable and writeable software. I guess you know that.

@SimonSchick
Copy link

I played around with it, there is no naming conflict with the delete keyword.

@Tyler-Murphy
Copy link

I also think this is a good idea.

Would a pull request for the following change be accepted?

request.del = verbFunc('del')
request['delete']= request.del

I see that PR #16 was rejected in 2011, but I don't think there's any problem using reserved keywords as object property names. http://stackoverflow.com/questions/7022397/using-reserved-words-as-property-names-revisited

@Tyler-Murphy
Copy link

It looks like #2175 resolves this.

@simov
Copy link
Member

simov commented Apr 28, 2016

Yep, I though this was going to be automatically closed but since the other PR was closed instead of merged this issue was left open.

@simov simov closed this as completed Apr 28, 2016
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

No branches or pull requests

4 participants