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

Add support for detecting DEPRECATION: warnings #468

Closed

Conversation

mhaylock
Copy link
Contributor

@mhaylock mhaylock commented Jul 2, 2016

These warnings aren't build errors (the build is still successful), so we need to avoid treating such DEPRECATION: lines as indicating a build error occurred.

This fixes #464 - a problem I've encountered with a new project using ember-cli-rails 0.7.4, Ember CLI 2.6.2 and ember-cli-rails-addon 0.7.0 (which is the source of the deprecation warning).

A different solution has been presented in PR #465 - this PR differs in that it will still detect build errors that occur even when a deprecation warning is present. It also ensures the build error files is still cleared between builds.

Thanks to @victor95pc for your work on #465 - it helped steer me in the right direction for fixing this :)

For the curious (because this is something I wondered myself) - the STDERR output of ember build is used to detect a build error instead of the exit status because in development mode the build is run with --watch and it never exits for there to be an exit status to assess.

"DEPRECATION: A warning",
" at AStackTrace",
"", # Blank line
"SyntaxError: things went wrong"

Choose a reason for hiding this comment

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

Put a comma after the last item of a multiline array.

@mhaylock
Copy link
Contributor Author

mhaylock commented Jul 2, 2016

The CI failures on this PR are due to the fact that those specs are also broken on master - the specs are still testing Rails 3.2 but the code does not support it. I've created a separate PR to fix this: #469.

@victor95pc
Copy link

Thx @mhaylock, my solution was not good enough? Can you explain a little more?

@mhaylock
Copy link
Contributor Author

mhaylock commented Jul 3, 2016

Hi @victor95pc, thanks again for your solution. There are couple of problems it doesn't address, but would be simple enough to fix. Apologies if I've caused any offence by presenting my own solution.

As you've requested further explanation I'll comment on your PR to clarify where I see there being some fixes required.

@victor95pc
Copy link

No problem at all, I take about 30 minutes to create that solution, I didn't thought too much about all use cases, I just thought my solution was more clean but yours cover better all use cases, thanks for giving your time on this awesome project I love. .

These warnings aren't build errors (the build is still successful), so
we need to avoid treating such DEPRECATION lines as indicating a build
error occured.
Address multiline array formatting issues highlighted by HoundCI.
@mhaylock mhaylock force-pushed the deprecation-warning-detection branch from 6ccd156 to e3eb29a Compare July 5, 2016 23:11
@mhaylock
Copy link
Contributor Author

mhaylock commented Jul 5, 2016

Thanks to @seanpdoyle merging #469 I've been able to rebase this PR and now the specs are passing.

@seanpdoyle
Copy link
Contributor

@mhaylock I've cleaned up and squashed your commits into 448bae9.

Thanks for this PR!

@mhaylock
Copy link
Contributor Author

mhaylock commented Jul 8, 2016

Excellent, thanks @seanpdoyle!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ember cli rails breaks when deprecations(warnings) are found on Ember
4 participants