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

moved errorCount to beginning to account for errors in all themes, no… #6171

Closed
wants to merge 0 commits into from

Conversation

bka
Copy link
Contributor

@bka bka commented Aug 15, 2016

For deployment, we need bin/magento to exit with a return code != 0 to indicate if something went wrong.

As far as I am aware this issue is at least related to following other issues:

The above pull request already addresses the issue that \Magento\Framework\Console\Cli::RETURN_FAILURE will be used if $this->errorCount > 0.
However, the current develop branch sets $this->errorCount = 0; for every iteration, which clears errors from previous themes.
I would suggest moving the errorCount to the beginning of deploy function, to account for all errors in all steps, because otherwise it gets reset everytime and only the last step in the loop will count.

Preconditions

  1. Tested on Magento 2.1.0 latest dev version on 903dcbb

Steps to reproduce

  1. Produce a syntax error in less, e.g. by creating an empty less: mkdir -p app/design/frontend/Magento/luma/Magento_Email/web/css/ && touch app/design/frontend/Magento/luma/Magento_Email/web/css/email.less
  2. Run bin/magento setup:static-content:deploy
  3. See error in output: e.g. Successful: 2138 files; errors: 1

Expected result

  1. Status code echo $? should be !=0 after running task

Actual result

  1. Status code echo $? is 0

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Aug 15, 2016

CLA assistant check
All committers have signed the CLA.

@bka
Copy link
Contributor Author

bka commented Aug 25, 2016

Sorry, closed in favour of having a separate branch for it, recreated here: #6349

magento-engcom-team pushed a commit that referenced this pull request Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants