-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Git based app blueprints #2103
Git based app blueprints #2103
Conversation
NICE! |
}) | ||
}); | ||
}).catch(function(err){ | ||
throw err; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why re-throw?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, just leftover from dev. It can go away.
looks great, mind adding some tests (so we don't accidentally regress)? |
@@ -137,6 +137,7 @@ | |||
"rimraf": "^2.2.8", | |||
"rsvp": "^3.0.13", | |||
"semver": "^3.0.1", | |||
"temp": "^0.8.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No real objection here, but can we consolidate on a single tmp library? We currently are using tmp-sync
(testing) and quick-temp
(used in broccoli processes)...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but why only have one? We should not discriminate, NPM is all about including all possible libraries for a task!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
quick-temp
puts them in ./tmp
but I don't want to pollute user space with temp folders for app generation. temp
uses whatever the system identifies as a system-wide temp location and cleans up after itself. quick-temp
makes sense for generation once the app exists, but I think it's icky before the app exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What we really need to do is write our own temp module that does exactly what we need...
188a3d4
to
03f27aa
Compare
@@ -16,6 +17,19 @@ var allowedWorkOptions = { | |||
everywhere: true | |||
}; | |||
|
|||
// extend nopt to recognize 'gitUrl' as a type | |||
nopt.typeDefs.gitUrl = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
neat!
03f27aa
to
b8a9e7f
Compare
@trek LGTM, let me know when it travis is happy: https://travis-ci.org/stefanpenner/ember-cli/builds/36371014 |
53a4917
to
5db3b4f
Compare
Looks good now. Needed to add the git checkout to the slow tests: https://travis-ci.org/stefanpenner/ember-cli/builds/36435142 |
37564dd
to
032a9bf
Compare
Extends `ember new'`s --blueprint option to accept git remotes in either git@someservice.com:app-blueprint-test.git or https://someservice.com/trek/app-blueprint-test.git style git remote urls.
032a9bf
to
9471422
Compare
Fixed a typo https://travis-ci.org/stefanpenner/ember-cli/builds/36496366 is still good |
WIP: Git based app blueprints
Extends
ember new'
s--blueprint
option to accept git remotes in eithergit@someservice.com:app-blueprint-test.git
orhttps://someservice.com/trek/app-blueprint-test.git
style git remote urls.Not a huge fan of how this is tested, but short of mocking I can't think of any way to truly test without doing an actual
git clone