Skip to content
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

Use build stages to fail early... #255

Merged
merged 2 commits into from
Dec 5, 2017

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Dec 5, 2017

No description provided.

@Turbo87
Copy link
Member

Turbo87 commented Dec 5, 2017

fancy, I definitely support this 👍

package.json Outdated
@@ -25,7 +25,9 @@
"scripts": {
"build": "ember build",
"start": "ember server",
"test": "ember try:each"
"test": "ember try:each",
"test:default": "ember test",
Copy link
Member

@Turbo87 Turbo87 Dec 5, 2017

Choose a reason for hiding this comment

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

I prefer test: ember test and test:all: ember try:each 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

ya, same, I'll update to that

@rwjblue rwjblue force-pushed the attempt-multi-stage-build branch 2 times, most recently from 3c12b38 to a2c3db9 Compare December 5, 2017 18:26
@rwjblue
Copy link
Member Author

rwjblue commented Dec 5, 2017

@Turbo87 - OK, I'm done playing around with this. Ready for review again...

@@ -36,22 +61,7 @@ before_install:
- export PATH=$HOME/.yarn/bin:$PATH

install:
- yarn install --no-lockfile
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed this because ember try:one (when useYarn: true is specified in config) already does yarn install --no-lockfile --ignore-engines when it is installing the scenario.

Copy link
Member

Choose a reason for hiding this comment

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

if it is runnin yarn install --no-lockfile and node_modules already exists, are you sure it updates all deps to the latest possible versions?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, I'm unsure. I'll add an explicit job to the first stage that is explicity testing transitive deps with the default set ("current" ember-source).

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, updated to add a single --no-lockfile job to the first stage. This way we don't have to ignore the lockfile for all jobs, and hopefully with this setup we can easily identify failures due to transient deps vs failures due to actual repo changes.

@rwjblue rwjblue changed the title [WIP] Use build stages to fail early... Use build stages to fail early... Dec 5, 2017
package.json Outdated
"test": "ember try:each"
"test": "ember test",
"test:all": "ember try:each",
"lint": "eslint addon-test-support config tests"
Copy link
Member

Choose a reason for hiding this comment

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

this seems to be missing *.js

Copy link
Member Author

Choose a reason for hiding this comment

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

Great point, I'll copy from my other PR in ember-cli.

@@ -36,22 +61,7 @@ before_install:
- export PATH=$HOME/.yarn/bin:$PATH

install:
- yarn install --no-lockfile
Copy link
Member

Choose a reason for hiding this comment

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

if it is runnin yarn install --no-lockfile and node_modules already exists, are you sure it updates all deps to the latest possible versions?

@rwjblue rwjblue merged commit b3c911c into emberjs:master Dec 5, 2017
@rwjblue rwjblue deleted the attempt-multi-stage-build branch December 5, 2017 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants