-
Notifications
You must be signed in to change notification settings - Fork 30k
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
Improving code readability on lib/_http_agent.js #83
Improving code readability on lib/_http_agent.js #83
Conversation
|
||
if (freeLen) { | ||
if (avaliableFreeSockets) { |
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.
typo: avaliableFreeSockets
-> availableFreeSockets
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.
it's done consistently though, haha ^^
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.
Fixed 🎉
// we have a free socket, so use that. | ||
var socket = this.freeSockets[name].shift(); | ||
debug('have free socket'); | ||
|
||
var hasFreeSockets = !this.freeSockets[name].length |
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.
Could this be accomplished with a comment rather than creating another var
to keep track of? I find the // don't leak
to be more sloppy than this particular change.
I don't like this pattern... but I applaud the effort of improving readability and variable naming as you have above. 👍
Closing as the discussion in #60 is ongoing on how to tackle this. |
- Make string API names consistent - Make string API coercion behavior consistent - Switch from number-remaining to number-copied results - Add UTF-16 string APIs - Add coercion APIs for number and bool - Update tests for API changes
[ci skip] PR-URL: ayojs/ayo#83 Reviewed-By: Jan Lehnardt <jan@apache.org> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: James Butler <james.butler@sandfox.co.uk> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Alex <alex@dytry.ch> Reviewed-By: <agentantelope+github@gmail.com> Reviewed-By: olivia <olivia@fastmail.com> Reviewed-By: srilq <git@srilq.email> Reviewed-By: Scott Trinh <scott@scotttrinh.com>
As per proposal #60, I have begun cleaning the code to create a more readable codebase.
I feel, and hope others agree, this will allow a more diverse and wider community to begin contributing and collaborating with io.js, reducing the learning curve and amount of developer documentation required for the codebase, equally making it easier to test and understand what is going on under the hook.
I will continue to clean the codebase into a self documenting one where possible, and encourage others to do the same.