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

Added Exception Handling for perhaps missing config param #1677

Merged
merged 28 commits into from
Sep 5, 2018
Merged

Added Exception Handling for perhaps missing config param #1677

merged 28 commits into from
Sep 5, 2018

Conversation

zuernBernhard
Copy link
Contributor

Sometimes deploy_path is not set and a cd into this folder is not possible. But the deployment can be successful without it.

Q A
Bug fix? Yes
New feature? No
BC breaks? BC ?
Deprecations? No
Fixed tickets fe4dd42#commitcomment-30224854

Do not forget to add notes about your changes to CHANGELOG.md

  • Add description under added/changed/fixed section.
  • Add reference to closed issues [#000].
  • Add link to issue in the end of document.

ryandev421 and others added 17 commits May 3, 2018 08:55
* moving the name column to the last position to show full name

- bug fix: #1579

* add changelog

* update changelog.md

* update CHANGELOG.md

* modify command to find http user

* modify command to find http user

* modify wrong ticket url
* Remove magento:enable from Magento 2 recipe

Magento's module states are stored in app/etc/config.php. This task
ignored that and blindly enabled all installed modules.

* Mention the removal of magento:enable in the changelog
* Added option cleanup_tty to allow allocation when using sudo

* Added comment for new option cleanup_tty to CHANGELOG
* Change drupal:settings task to drupal:configure

* Drupal 7 recipe - Set template file path

* Changelog
#1624)

* Fixed once() tasks that where being run multiple times with ParallelExecutor

* CHANGELOG: reorder issue

* Add test case for parallel deploy on ->once() task
* Added option cleanup_tty to allow allocation when using sudo

* Added comment for new option cleanup_tty to CHANGELOG

* Added cleanup_use_sudo option to all commands in cleanup.php
* Add Prestashop 1.6 recipe

* Update changelog
* Sleep between progress status calls to prevent 100% CPU usage

* Add changelog entry

* Typo in changelog entry

* Update ParallelExecutor.php

* Update ParallelExecutor.php
* Detect CI user

* Update CHANGELOG.md
* deploy:writable: chgrp doesn't need http_user

* Update CHANGELOG.md
Sometimes deploy_path is not set and a cd into this folder is not possible. But the deployment can be successful without it.
@@ -77,7 +77,13 @@
}
}

cd('{{deploy_path}}');
try {
Copy link
Member

Choose a reason for hiding this comment

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

I think it's better to check with if (has('deploy_path')) {

cd('{{deploy_path}}');
} catch (\Exception $e) {
// Deploy_path is not set
writeln("➤ Deploy_Path is not set. -> won't cd into it");
Copy link
Member

Choose a reason for hiding this comment

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

And I thing we don't need logging here.

@zuernBernhard
Copy link
Contributor Author

done - fixed according to your feedback. still works :) can be merged ?

@antonmedv
Copy link
Member

Cool! Please update changelog too.

@zuernBernhard
Copy link
Contributor Author

oh sry - forgot that. hope its ok now

https://github.com/zuernBernhard/deployer/blob/patch-1/CHANGELOG.md

@antonmedv antonmedv merged commit 26c3400 into deployphp:master Sep 5, 2018
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.