Issue #3313: DRY up etag and wetag code. #3314
Conversation
lib/utils.js
Outdated
| * @api private | ||
| */ | ||
|
|
||
| function setTag(weak) { |
There was a problem hiding this comment.
Please place the non-exported function with the others at the bottom of the file.
lib/utils.js
Outdated
| ? new Buffer(body, encoding) | ||
| : body; | ||
|
|
||
| return etag(buf, {weak: weak}); |
There was a problem hiding this comment.
All net new code should be written in StandardJS style per https://github.com/expressjs/express/blob/master/Collaborator-Guide.md#prs-and-code-contributions
…ution per feedback.
|
Submitted new version of code contribution. Per feedback, I moved the helper function I created to the bottom of the file, ran the JS Standard tool against my contribution and made changes per its recommendations, but only to the code I touched. Note: the JS Standard tool found a number of issues with the code in ./lib/utils.js that do NOT relate to the code refactoring I did. |
|
Awesome! And yea, it is not expected at this time that the files will pass; changing the style will cause almost every line to change, so rather than wholesale change every line all of a sudden, breaking all outstanding PRs, just styling changed lines is what the plan is. |
lib/utils.js
Outdated
| * @api private | ||
| */ | ||
|
|
||
| function setTag (weak) { |
There was a problem hiding this comment.
I just noticed this is "setTag" instead of like "setETag". Is the missing "e" a typo?
…er function name.
|
Per your feedback, I've updated the name of the helper function from "setTag" to "setETag." Thanks for pointing this out. Cheers. |
After implementing the code, all tests passed and performance benchmarks checked out.