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

feat: only use server listen to detect port #13

Merged
merged 1 commit into from
Jan 17, 2017
Merged

Conversation

fengmk2
Copy link
Member

@fengmk2 fengmk2 commented Jan 17, 2017

use npm scripts instead of Makefile

use npm scripts instead of Makefile
@mention-bot
Copy link

@fengmk2, thanks for your PR! By analyzing the history of the files in this pull request, we identified @xudafeng, @zenzhu and @gaearon to be potential reviewers.

@fengmk2
Copy link
Member Author

fengmk2 commented Jan 17, 2017

@xudafeng 原来的实现太复杂了,只需要 server listen 即可,不需要客户端 connect

loop(port);
server.on('error', err => {
debug('listen %s error: %s', port, err);
port = 0;
Copy link
Member Author

Choose a reason for hiding this comment

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

失败一次之后马上监听随机端口,确保操作系统分配成功。

return promise;
}
};
server.listen({ port }, () => {
Copy link
Member

Choose a reason for hiding this comment

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

4 不支持这个解构

Copy link
Member

Choose a reason for hiding this comment

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

喔,看错,不是函数签名

@xudafeng xudafeng requested a review from ziczhu January 17, 2017 09:10
@xudafeng
Copy link
Member

@fengmk2 👍

@xudafeng xudafeng merged commit 0984b32 into master Jan 17, 2017
@xudafeng xudafeng deleted the listen-only branch January 17, 2017 09:49
@fengmk2
Copy link
Member Author

fengmk2 commented Jan 17, 2017

@xudafeng 发一个 minor 版本

@xudafeng
Copy link
Member

done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants