Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Allowing --no-build flag during skyux e2e #262

Merged
merged 4 commits into from
Sep 5, 2017
Merged

Conversation

Blackbaud-BobbyEarl
Copy link

No description provided.

@codecov-io
Copy link

codecov-io commented Aug 28, 2017

Codecov Report

Merging #262 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #262   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          58     58           
  Lines        1340   1349    +9     
  Branches      197    200    +3     
=====================================
+ Hits         1340   1349    +9
Flag Coverage Δ
#builder 100% <100%> (ø) ⬆️
#runtime 100% <ø> (ø) ⬆️
#srcapp 100% <ø> (ø) ⬆️
Impacted Files Coverage Δ
utils/host-utils.js 100% <100%> (ø) ⬆️
cli/e2e.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 293530a...3747f64. Read the comment docs.

cli/e2e.js Outdated
logger.info('Running build...');
build(argv, skyPagesConfig, webpack)
.then(stats => {
logger.info('Build complete.');
console.log(stats.toJson().chunks);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to leave the console log in here?

cli/e2e.js Outdated
if (argv.build === false) {
logger.info('Skipping build step');
return resolve({
metadata: fs.readJsonSync('dist/metadata.json')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know fs likes to throw errors around. Are we fairly confident that metadata.json will always exist? ie, we don't need to wrap this in a try/catch?

@Blackbaud-BobbyEarl
Copy link
Author

Great feedback @Blackbaud-SteveBrush. Should be addressed in latest. I haven't seen rampant fs errors, but I did add a check to verify the file exists before trying to read.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants