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

Improving code readability on lib/_http_agent.js #83

Closed
wants to merge 2 commits into from
Closed

Improving code readability on lib/_http_agent.js #83

wants to merge 2 commits into from

Conversation

imjacobclark
Copy link

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.


if (freeLen) {
if (avaliableFreeSockets) {

Choose a reason for hiding this comment

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

typo: avaliableFreeSockets -> availableFreeSockets

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 ^^

Copy link
Author

Choose a reason for hiding this comment

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

Fixed 🎉

@imjacobclark imjacobclark changed the title Improving code readability Improving code readability on lib/_http_agent.js Dec 5, 2014
// we have a free socket, so use that.
var socket = this.freeSockets[name].shift();
debug('have free socket');

var hasFreeSockets = !this.freeSockets[name].length
Copy link

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. 👍

@imjacobclark
Copy link
Author

Closing as the discussion in #60 is ongoing on how to tackle this.

boingoing pushed a commit to boingoing/node that referenced this pull request Apr 6, 2017
 - 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
forivall pushed a commit to forivall/node that referenced this pull request Oct 7, 2017
[ci skip]

PR-URL: ayojs/ayo#83
Reviewed-By: Jan Lehnardt <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: James Butler <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Alex <[email protected]>
Reviewed-By: <[email protected]>
Reviewed-By: olivia <[email protected]>
Reviewed-By: srilq <[email protected]>
Reviewed-By: Scott Trinh <[email protected]>
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.

3 participants