RC > Instrument different directory for libraries #448
Conversation
Codecov Report
@@ Coverage Diff @@
## rc-2.0.0 #448 +/- ##
============================================
+ Coverage 99.42% 99.42% +<.01%
============================================
Files 72 72
Lines 1899 1903 +4
Branches 297 298 +1
============================================
+ Hits 1888 1892 +4
Misses 11 11
Continue to review full report at Codecov.
|
| @@ -20,7 +20,13 @@ function getWebpackConfig(skyPagesConfig, argv) { | |||
| const runCoverage = (!argv || argv.coverage !== false); | |||
| skyPagesConfig.runtime.includeRouteModule = false; | |||
| const ENV = process.env.ENV = process.env.NODE_ENV = 'test'; | |||
| const srcPath = path.resolve(process.cwd(), 'src', 'app'); | |||
|
|
|||
There was a problem hiding this comment.
This is probably more non-blocking feedback, but it stood out to me that we now have to check for argv twice. What are your thoughts on slightly refactoring to something like the following (untested)?
const argvCoverage = argv ? argv.coverage : '';
const srcPathUnresolved = [process.cwd(), 'src', 'app'];
const runCoverage = argvCoverage !== false;
if (argvCoverage === 'library') {
srcPathUnresolved.push('public');
}
const srcPath = path.resolve(...srcPathUnresolved);
skyPagesConfig.runtime.includeRouteModule = false;
const ENV = process.env.ENV = process.env.NODE_ENV = 'test';
| srcPath = path.resolve(process.cwd(), 'src', 'app', 'public'); | ||
| } else { | ||
| srcPath = path.resolve(process.cwd(), 'src', 'app'); | ||
| } |
There was a problem hiding this comment.
@Blackbaud-BobbyEarl I agree about the argv.coverage checks! I updated the logic, there.
For the srcPath, I personally think writing out the paths long-hand for each condition is more readable, since the reader doesn't need to parse out the array.push behavior for each condition. Also, keeping them separate allows more flexibility if we want to change the paths in the future (for example, if the library's source path wasn't a descendant of the SPA's source path). Just my opinion. Thoughts on that?
* RC > Removed all references to SKY UX, changed dependency structure (blackbaud#419) * RC > Updated TSLint rules (blackbaud#422) * Removed legacy omnibar (blackbaud#420) * RC > Config params as an object; always decode URL params (blackbaud#421) * RC > Always provide `envId` regardless of permission scope (blackbaud#427) * RC > Allow SPAs to bundle stylesheets that live outside of `src/app` (blackbaud#428) * RC > Adjusted dev dependencies (blackbaud#429) * RC > Fixed ts-helpers for build (blackbaud#434) * RC > Update from master (blackbaud#425) * RC > Removed global RxJS imports (blackbaud#438) * RC > Replaced error component with iframe (blackbaud#436) * RC > Removed SKY CSS import (blackbaud#443) * RC > Instrument different directory for libraries (blackbaud#448) * RC > Do not ignore Protractor Error 199 (blackbaud#435) * RC > Merged master (blackbaud#444) * RC > Merge master (blackbaud#454) * RC > Upgrade Angular, RxJS, TypeScript (blackbaud#495) * RC > Moved auth-client to peer dependency; fixed `skyux watch` (blackbaud#503) * Disabled webpack host check (blackbaud#505) * Replaced JSHint with ESLint. (blackbaud#506) * RC > Merge master (blackbaud#508) * RC > Changed name of NPM package (blackbaud#501)
skyux test --coverage library