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

Accept lists of ports #28

Merged
merged 4 commits into from
Feb 23, 2015
Merged

Accept lists of ports #28

merged 4 commits into from
Feb 23, 2015

Conversation

darthneko
Copy link
Contributor

Modified existing methods to work with a list of ports instead of a
starting port and ending port.

Modified existing methods to work with a list of ports instead of a
starting port and ending port.
@darthneko darthneko mentioned this pull request Jan 23, 2015
, async = require('async')
var net = require('net');
var Socket = net.Socket;
var async = require('async');
Copy link
Contributor

Choose a reason for hiding this comment

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

In general, I recommend sticking to the current project style when submitting pull requests. Changes like adding var or ; are just noise. And, if they were accepted, it would make this project have a mix of styles. Please remove these changes.

Implemented changes suggested. Cohesive code style and backwards
compatibility. New methods are named as the old method with a "inList"
suffix - for instance, findAPortNotInUseInList.
@darthneko
Copy link
Contributor Author

Revised changes.

@sholladay
Copy link

Any chance for detecting arrays from the original APIs instead of adding new methods?

Now the original API works with both arrays and start/end ports.
Detection on which method to use is based on argument number.
@darthneko
Copy link
Contributor Author

Yes, now both functionalities work with the original API.

@sholladay
Copy link

Cool. If it gets a blessing from @EndangeredMassa we can close the related issue.

@darthneko
Copy link
Contributor Author

Still waiting for an update on this.

@sholladay
Copy link

Same. If the project maintainer does not respond or merge this within the next few days, I will carve out some time to test it so we can resolve the bounty.

@shinnn
Copy link
Collaborator

shinnn commented Feb 23, 2015

I started reviewing.

shinnn added a commit that referenced this pull request Feb 23, 2015
@shinnn shinnn merged commit 3a271ee into baalexander:master Feb 23, 2015
laggingreflex added a commit to laggingreflex/node-portscanner that referenced this pull request Nov 18, 2016
Revert "Merge pull request baalexander#28 from darthneko/master"

This reverts commit 3a271ee, reversing
changes made to c2092ce.
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.

5 participants