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

adjust registerer and listen stage #58

Closed
wants to merge 5 commits into from
Closed

Conversation

mlboy
Copy link
Collaborator

@mlboy mlboy commented Jun 18, 2020

Describe what this PR does / why we need it

After adding the auto port, I think the code has become somewhat inconsistent, so I put listen in the serve() stage.

Does this pull request fix one issue?

Describe how you did it

Make code neat

Describe how to verify it

run some example

Special notes for reviews

@mlboy
Copy link
Collaborator Author

mlboy commented Jun 18, 2020

pr 主要做了以下几件事:
1,调整了servers 注册、注销时机(将注销放在了stop中,将注册测defer挪到了eg.go外的线程中)
2,调整了注册服务的关闭时机(放在了最后)
3,统一调整了server下的 listen时机 (统一放在了serve方法中,newServer不在承担打开监听的事)

@gorexlv gorexlv added the to-review need review label Jun 19, 2020
@askuy
Copy link
Contributor

askuy commented Jun 21, 2020

pr 主要做了以下几件事:
1,调整了servers 注册、注销时机(将注销放在了stop中,将注册测defer挪到了eg.go外的线程中)
2,调整了注册服务的关闭时机(放在了最后)
3,统一调整了server下的 listen时机 (统一放在了serve方法中,newServer不在承担打开监听的事)

Rebase一下提交记录,在一次提交里说明修改内容。谢谢。

@mlboy
Copy link
Collaborator Author

mlboy commented Jun 21, 2020

pr 主要做了以下几件事:
1,调整了servers 注册、注销时机(将注销放在了stop中,将注册测defer挪到了eg.go外的线程中)
2,调整了注册服务的关闭时机(放在了最后)
3,统一调整了server下的 listen时机 (统一放在了serve方法中,newServer不在承担打开监听的事)

Rebase一下提交记录,在一次提交里说明修改内容。谢谢。

done f85ad9e

@gorexlv
Copy link
Member

gorexlv commented Jun 22, 2020

先创建一个issue讨论一下吧,这个修改在某些场景下有些风险。

暂时先转为Draft.

Thanks

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

Successfully merging this pull request may close these issues.

None yet

3 participants