Overhaul on upstream retrieval. #27
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
illepic
previously requested changes
Nov 29, 2017
generators/app/index.js
Outdated
| var exec = require('child_process').execSync; | ||
| var fs = require('fs-extra'); | ||
| var path = require('path'); |
There was a problem hiding this comment.
If we're cool with const down below (node 6+) then we're cool with const up here.
| @@ -18,8 +21,7 @@ module.exports = Generator.extend({ | |||
| 'Welcome to the remarkable ' + chalk.red('PatternLabStarter') + ' generator! ' + this.pkg.version + '\nPlease be in the folder you want files in now.' | |||
There was a problem hiding this comment.
This is still going to be pulling down PLS v9 for awhile.
generators/app/index.js
Outdated
| }, | ||
|
|
||
| configuring: function () { | ||
|
|
||
| }, | ||
|
|
||
| default: function () { | ||
| var dest = options.themePath ? path.resolve(process.cwd(), options.themePath) : './'; | ||
| const release = options['release'] || 'master' | ||
| const release_path = release + '.tar.gz'; |
There was a problem hiding this comment.
const release_path = `${release}.tar.gz`;
generators/app/index.js
Outdated
| const release_path = release + '.tar.gz'; | ||
| const compressed = _.last(_.split(release_path, '/')); | ||
| const decompressed = _.replace('particle-' + release, '/', '-'); | ||
| const download_url = 'https://github.com/phase2/particle/archive/' + release_path; |
There was a problem hiding this comment.
See above for template literal approach to this.
generators/app/index.js
Outdated
| const decompressed = _.replace('particle-' + release, '/', '-'); | ||
| const download_url = 'https://github.com/phase2/particle/archive/' + release_path; | ||
| const dest = options.themePath ? path.resolve(process.cwd(), options.themePath) : './'; | ||
| const themeName = 'patternlab'; |
There was a problem hiding this comment.
I'm not trying to bring this to generator-particle just yet, simply continue to pull down the PLS 9 repo. I suppose you could have created a whole new repo... meh.
generators/app/index.js
Outdated
|
|
||
| // @todo replace tarball retrieval & extraction with a Node library. | ||
| try { | ||
| this.log('Retrieving template from ' + download_url + '...'); |
There was a problem hiding this comment.
You already know that I'm going to suggest a template literal.
generators/app/index.js
Outdated
| try { | ||
| this.log('Retrieving template from ' + download_url + '...'); | ||
| exec([ | ||
| 'curl --fail --silent -OL ' + download_url, |
There was a problem hiding this comment.
This one will be a literal template.
generators/app/index.js
Outdated
| this.log('Retrieving template from ' + download_url + '...'); | ||
| exec([ | ||
| 'curl --fail --silent -OL ' + download_url, | ||
| 'tar -xzf ' + compressed |
generators/app/index.js
Outdated
| } catch(error) { | ||
| console.error('Could not "mkdir -p" the themePath.'); | ||
| this.log.error('Could not create the theme path: ' + error); |
generators/app/index.js
Outdated
| fs.renameSync(decompressed, themePathFull) | ||
| } catch(error) { | ||
| console.error(error); | ||
| this.log.error('Could not move theme into position: ' + error); |
illepic
reviewed
Nov 30, 2017
| @@ -22,6 +26,10 @@ Extras can be installed with: | |||
| yo pattern-lab-starter:extras | |||
There was a problem hiding this comment.
Not sure what you're getting at?
grayside
dismissed
illepic’s stale review
Because I addressed all understandable feedback.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A few little changes, but the heart of this is to minimize the amount of exec'd commands. I wanted to use a library such as tarball-extract (alluded to in #25) but that project depends on an ancient release of a node wget library that can't follow redirects, which immediately slams against Github's infrastructure.
This change adds a new --release flag to facilitate testing/using the generator with code other than master on the particle project.
There is also some general robustness improvements, such as failing earlier if curl asplodes, removing extra file copy and delete operations, and supporting the generator recursively creating the base directory of the theme in case someone specified a deeply nested location.
With this change I suppose I'll follow up with a v3.1.0 release.