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.

Sign up for GitHub

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

Jump to bottom

let the http route read server name from the config #2461

Merged
merged 5 commits into from
Sep 8, 2020
Merged

let the http route read server name from the config #2461

merged 5 commits into from
Sep 8, 2020

Conversation

nashgao
Copy link
Contributor

@nashgao nashgao commented Sep 8, 2020

No description provided.

@limingxinleo limingxinleo requested a review from Reasno September 8, 2020 06:02
@limingxinleo
Copy link
Member

这样会增加新的依赖进来 @Reasno

感觉不是很合适

@nashgao
Copy link
Contributor Author

nashgao commented Sep 8, 2020 •
edited
Loading

不过那个server name http 就是写死了
如果改了servername这个 http route observable就没法用了 就只能通过class_map重写才可以了

@limingxinleo
Copy link
Member

我觉得，构造函数再传一个 $serverName 默认是http会更加合理。

@nashgao
Copy link
Contributor Author

nashgao commented Sep 8, 2020

已修改

@Reasno
Copy link
Member

Reasno commented Sep 8, 2020

我看修改完的版本没有什么问题。

Reasno
Reasno previously approved these changes Sep 8, 2020
View reviewed changes
@limingxinleo limingxinleo merged commit 5824747 into hyperf:master Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@limingxinleo limingxinleo limingxinleo approved these changes

@Reasno Reasno Reasno left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nashgao @limingxinleo @Reasno