Fix 'npm run gulp updatePackageDependencies' #2766
Conversation
This commit improves the updatePackageDependencies gulp task so that hopefully it will work for all three teams needing to update package.json to point at new runtime dependancies in hopefully a safe way. Fixes/improvements: * Make the task work again since gulp no longer likes the command line argument hack that we used to have * Add support for computting checksums * Add verification that the primary and fallback URLs have the same hash and both work
Codecov Report
@@ Coverage Diff @@
## master #2766 +/- ##
==========================================
- Coverage 65.29% 65.27% -0.03%
==========================================
Files 102 102
Lines 4440 4443 +3
Branches 641 641
==========================================
+ Hits 2899 2900 +1
- Misses 1356 1358 +2
Partials 185 185
Continue to review full report at Codecov.
|
| } | ||
| } | ||
|
|
||
| if (! /^[0-9]+\.[0-9]+\.[0-9]+$/.test(newVersion)) { |
There was a problem hiding this comment.
We could also do a semver check here
There was a problem hiding this comment.
I don't think there is anything in 'semver' that supports searching for a semver in a string, and since we always want the input version to be subsequently searchable, I don't think it makes sense to use the semver library for this.
| eventStream.subscribe((event: Event.BaseEvent) => { | ||
| switch (event.constructor.name) { | ||
| case Event.DownloadFailure.name: | ||
| console.log("Failed to download: " + (<Event.DownloadFailure>event).message); |
There was a problem hiding this comment.
Like in the other places in the code, we could use a single eventStream that is passed to all these functions and in this file we could post the appropriate events to this stream and separately create a "ConsoleObserver" or "UpdatePackageDependencies" observer that subscribes to this event stream and does the logging accordingly.
There was a problem hiding this comment.
While I think the eventStream pattern that we created in other places make a lot of sense for enabling unit tests, I don't really see a benefit in this case since unit tests don't seem to make sense for a gulp task that produces output that would be code reviewed.
3 participants
This commit improves the updatePackageDependencies gulp task so that hopefully it will work for all three teams needing to update package.json to point at new runtime dependancies in hopefully a safe way.
Fixes/improvements: