add ability to run deploy on static client #30
Conversation
Blackbaud-TimPepper
requested review from
Blackbaud-BobbyEarl and
Blackbaud-BrandonJones
lib/assets.js
Outdated
|
|
||
| if (isStaticClient) { | ||
| // This should just return the single project bundle | ||
| const bundle = glob.sync(path.join(dist, 'bundles', '*.js')); |
There was a problem hiding this comment.
Some of our bundles definitely have multiple files (regular vs minified). I would adjust this to publish anything *.* inside the bundles folder.
There was a problem hiding this comment.
Would we want all the files? Currently the following logic receives a single path. What would be the best path forward for handling multiple files?
lib/assets.js
Outdated
| if (isStaticClient) { | ||
| return glob.sync(path.join(dist, 'bundles', '*.js')) | ||
| .map(bundle => { | ||
| const bundlePath = bundle.split('/'); |
There was a problem hiding this comment.
I would suggest using path.getbasename() method instead. https://nodejs.org/api/path.html#path_path_basename_path_ext
Blackbaud-TimPepper
changed the base branch from
add-logSkyuxVersion-prop
to
master
Codecov Report
@@ Coverage Diff @@
## master #30 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 7 7
Lines 148 162 +14
Branches 15 19 +4
=====================================
+ Hits 148 162 +14
Continue to review full report at Codecov.
|
lib/assets.js
Outdated
| let metadataPath = path.join(dist, 'metadata.json'); | ||
|
|
||
| if (isStaticClient) { | ||
| return glob.sync(path.join(dist, 'bundles', '*.js')) |
There was a problem hiding this comment.
This looks really good to me @Blackbaud-TimPepper. I think I have one small last change, I would go ahead and add a line (around line 12) like const bundlesDist = path.join(dist, 'bundles') and use that here and line 73. That way if we ever change the folder, it's only in one spot.
Saving bundles path.
No description provided.