ember-source related cleanup #14457
ember-source related cleanup #14457
Conversation
| bower.json | ||
| bower_components | ||
| config | ||
| # dist |
There was a problem hiding this comment.
What is this line suppose to do?
There was a problem hiding this comment.
It's just a comment. In the future we might want to only ship certain things inside /dist
There was a problem hiding this comment.
Then let's include more info. 😊
| config | ||
| # dist | ||
| ember-cli-build.js | ||
| ember-source-2.10.0-alpha.1.tgz |
There was a problem hiding this comment.
Should this be something like ember-source-*.tgz or will be necessary to update after each version?
There was a problem hiding this comment.
Removed, was a left over from copying the ls output
|
If we're omitting most of the files from npm, we may want to consider using the package.json "files" configuration over npmignore. |
|
@mitchlloyd We did investigate and consider that. @locks probably remembers the reasoning for that not being a thing. |
|
From what I understand, |
| @@ -0,0 +1,3 @@ | |||
| #!/usr/bin/env bash | |||
|
|
|||
| ember build --production && npm publish | |||
There was a problem hiding this comment.
Turn this into a JavaScript file and execSync and this will be portable. I'd prefer not to make the release process non-portable.
There was a problem hiding this comment.
Do you have an execSync example handy?
There was a problem hiding this comment.
There was a problem hiding this comment.
--production is not a valid option. It should be ember build --environment production or ember build -prod.
| # In the future we will likely want to restrict exactly | ||
| # which parts of `/dist` we want to include. | ||
| # | ||
| # dist |
There was a problem hiding this comment.
I feel like we should split this out to the very bottom of the file. Especially since it's commented to begin with. It can just be presented as a documentation of:
Currently we include the entire
distfolder, but at some point we may wish to be more selective about what we include.
|
@rwjblue With one last change I believe this is ready to land. The only thing it doesn't currently have is a test for the assets which get published to ensure that all of the pieces we need get published. I feel like that test likely belongs on the Ember CLI side and not part of this repo so I'm in favor of landing this. @locks Thank you for getting this to work in a portable manner and helping to push this over the line. <3 |
| var execSync = require('child_process').execSync; | ||
|
|
||
| execSync("ember build --environment production"); | ||
| execSync("npm publish"); |
There was a problem hiding this comment.
This actually seems a bit scary to auto-trigger. I feel like we just need:
execSync("ember build --environment production");
And that it should run as part of the prepublish hook. I still like it as a separate script because I'm ~100% sure that it will grow.
Fixes #14434. See issue for checklist.
prepublishscript in ember to doember build -prod.npmignore(we are including waaaaaayyyyy to much in the tar.gz right now)ember-cli-htmlbarsfor the ember-source name change Update ember-addon-main.js from ember-core to ember-source ember-cli/ember-cli-htmlbars#98execSyncfor release script