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

rename.. #1

Merged
merged 1 commit into from
Jan 21, 2017
Merged

rename.. #1

merged 1 commit into from
Jan 21, 2017

Conversation

zensh
Copy link
Member

@zensh zensh commented Jan 20, 2017

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 910c810 on issue/rename into 7127b80 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 5efac18 on issue/rename into 7127b80 on master.

@zensh
Copy link
Member Author

zensh commented Jan 20, 2017

@mushroomsir 按照 go 源码的习惯，error 一般小写开头

DavidCai1111
DavidCai1111 reviewed Jan 20, 2017
View reviewed changes
cookie.go
request: req,
writer: res,
req: r,
w: w,
Copy link
Member

Choose a reason for hiding this comment

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

w 写法改统一了，但是 req 和 r 依然有点不一样的样子

Copy link
Member

@DavidCai1111 DavidCai1111 Jan 20, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

感觉要不和 w 一样都统一为 r ? (http.HandleFunc("/bar", func(w http.ResponseWriter, r *http.Request) {}))

Copy link
Member Author

@zensh zensh Jan 20, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

小伙子你 simple 了，我特意用了 req，这样看源码方便理解。不信你看 golang 的 server 源码，都是req, res。另外还一个原因是 w 出现极少没关系，w 表明它是 writer，不是完整的 response，所以没用 res :)

Copy link
Member

Choose a reason for hiding this comment

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

🤔🤔🤔

@mushroomsir mushroomsir merged commit b39575b into master Jan 21, 2017
@mushroomsir mushroomsir deleted the issue/rename branch January 21, 2017 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DavidCai1111 DavidCai1111 DavidCai1111 left review comments

@mushroomsir mushroomsir Awaiting requested review from mushroomsir

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

Successfully merging this pull request may close these issues.
4 participants
@zensh @coveralls @DavidCai1111 @mushroomsir