docs(middleware): clarify MethodOverrideGetter return value#3059
docs(middleware): clarify MethodOverrideGetter return value#3059sonnemusk wants to merge 2 commits into
Conversation
Overridden methods such as "delete" did not match router http.Method* constants. Trim and uppercase the override value before assigning.
| req := c.Request() | ||
| if req.Method == http.MethodPost { | ||
| m := config.Getter(c) | ||
| m := strings.TrimSpace(config.Getter(c)) |
There was a problem hiding this comment.
I do not think fixing invalid http method (removing spaces or upper-casing) is middleware task.
There was a problem hiding this comment.
I think we could add comment config.Getter what it should output.
There was a problem hiding this comment.
Addressed in the latest commit: runtime normalization removed; Getter docs now state it should return a standard method name (e.g. http.MethodDelete).
There was a problem hiding this comment.
Addressed in the latest commit: runtime normalization removed; Getter docs now state it should return a standard method name (e.g. http.MethodDelete).
There was a problem hiding this comment.
Done — MethodOverrideGetter godoc now documents that it should return a standard method name such as http.MethodDelete / "DELETE", and that the middleware does not normalize case or trim spaces.
Per review: do not normalize override methods in middleware. Document that Getter should return a standard method name (e.g. http.MethodDelete).
sonnemusk
changed the title
|
Thanks — agreed. I dropped the upper-case/trim runtime change and only documented that `MethodOverrideGetter` should return a standard method name (e.g. `http.MethodDelete`). Please take another look. |
1ec77ec to
a4dd591
Compare
2 participants
Summary
Per review feedback: normalizing override methods (upper-case / trim) is not the middleware's job.
This PR only documents that
MethodOverrideGettershould return a standard HTTP method name (e.g.http.MethodDelete/"DELETE"). No runtime behavior change.Test plan