Move remaining helper functions to lib/ #1109
Conversation
paulmelnikow
commented
Sep 28, 2017
paulmelnikow
commented
- Clear the regular update cache between unit tests
- Clear the regular update cache between unit tests
# Conflicts: # server.js
lib/nexus-snapshot-version.js
Outdated
| return version.match(pattern); | ||
| } else { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
This could just do `return !!version && version.match(pattern);
lib/nexus-snapshot-version.js
Outdated
|
|
||
| module.exports = { | ||
| isNexusSnapshotVersion | ||
| }; |
There was a problem hiding this comment.
Maybe call this module nexus-version rather than nexus-snapshot-version
lib/nexus-snapshot-version.js
Outdated
| } | ||
|
|
||
| module.exports = { | ||
| isNexusSnapshotVersion |
There was a problem hiding this comment.
Just call this isSnapshotVersion, since "nexus" is redundant (it's already in the module name)
lib/nuget-badge-helpers.js
Outdated
| module.exports = { | ||
| mapNugetFeedv2, | ||
| mapNugetFeed | ||
| }; |
There was a problem hiding this comment.
Maybe call these "providers" and make a providers directory for them? "helpers" doesn't really fully describe what these functions do, since they actually serve the badges (so they're not just helpers).
There was a problem hiding this comment.
Right, it's true that they're not helpers.
I think #963 is a much better approach for refactoring services. I don't want to fortify or encourage this approach, and feel like making a folder would do that. How about I rename them from -badge-helpers to -provider?
There was a problem hiding this comment.
That sounds good to me! I'll revive #963 soon, when I have a bit more free time :)
|
|
||
| // Map from URL to { timestamp: last fetch time, interval: in milliseconds, | ||
| // data: data }. | ||
| let regularUpdateCache = Object.create(null); |
There was a problem hiding this comment.
I think we're on a new enough version of Node to use Map here, rather than an "object as a map".
|
|
||
| function makeSend(format, askres, end) { | ||
| if (format === 'svg') { | ||
| return function(res) { sendSVG(res, askres, end); }; |
There was a problem hiding this comment.
Use an arrow function:
return res => sendSVG(res, askres, end);
|
Thanks for reviewing! None of this is new code, and I prefer to avoid making changes as code is moved, since the changes are impossible to discern in the commit history. I'll update the names, and open a new PR for the rest! |
# Conflicts: # server.js
|
Nexus service tests are not passing locally, though the Nexus failure exists in e2ee910, when the code entered, so it's not related to this change. Also failing are github and amo, also unrelated. |
Address comments from #1109 and make several code-quality fixes which were caught by a local run of eslint-config-standard.
