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

Change default theme to Sage & add prompt to install bower packages. #186

Closed
wants to merge 2 commits into from

Conversation

brmullikin
Copy link
Collaborator

@wesleytodd for #185

  1. Changes the default theme to https://github.com/roots/sage/tree/8.5.0
  2. If a custom theme, prompt user about using Bower & install bower packages if yes

Prompt for Bower too, and install if yes && bower.json exists

Make sure bower is only called if there is a custom theme
Copy link
Owner

@wesleytodd wesleytodd left a comment

Choose a reason for hiding this comment

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

Other than these two small-ish changes, this looks great! Thanks!

@@ -255,7 +255,8 @@ function setupTheme(generator, config, done) {
themePackageJson = path.join(themePath, 'package.json');

var themeTaskFile = '',
themeTaskCmd = '';
themeTaskCmd = '',
BowerFile = 'bower.json';
Copy link
Owner

Choose a reason for hiding this comment

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

For consistency, can you use camel case for var names?

return done();
}

if (config.bower && fs.existsSync(BowerFile)) {
exec('bower install', function(err, stdout, sterr) {
Copy link
Owner

Choose a reason for hiding this comment

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

This will have to be refactored. If config.bower is true, it will result in running the lines at 287 before the bower install is complete. Probably needs flow control logic, maybe look at using https://www.npmjs.com/package/run-series?

@brmullikin
Copy link
Collaborator Author

@wesleytodd I removed the bower prompt for now so we could just merge in a new default theme.

@wesleytodd
Copy link
Owner

Looks good! I will pull it in and publish when I get home from work today. This will be a major version change, and so I will try to land the other pending PRs that are around as well. Thanks again!!

@brmullikin
Copy link
Collaborator Author

@wesleytodd np. I'll also look into adding the bower prompt back using run-series like you suggested, but won't be able to look at it until at least this weekend.

@wesleytodd
Copy link
Owner

Also added you as a collab here :)

@brmullikin brmullikin closed this Apr 24, 2019
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.

2 participants