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

Fixes #1572: Disable git mode warning with configuration #1882

Closed
wants to merge 1 commit into from

Conversation

greg-1-anderson
Copy link
Member

No description provided.

// Skip the warning for 'test' and 'live'
if (in_array($environment->id, ['test', 'live'])) {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use $environment->isDevelopment() for this logic. It'd also be shorter if you ask

if ($environment->isDevelopment()) {
    $this->validateConnectionMode($environment->get('connection_mode'));
}

@@ -120,6 +124,10 @@ protected function validateEnvironment($site, $environment)
*/
protected function validateConnectionMode($mode)
{
// Skip the warning if it is disabled
if ($this->getConfig()->get('hide_git_mode_warning')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this logic ought to apply directly to the if statement directly before the warning. If we add other outputs to this later we'll have to condense this anyway because it asks about a specific warning and this function won't necessarily always deal with this one warning.

if (!$this->getConfig()->get('hide_git_mode_warning') && ($mode === 'git')) {
    $this->log()->warning(

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure. I prefer the exit-early paradigm, but the variants you suggest are also clear and have merits as described. I'll alter as proposed.

@greg-1-anderson
Copy link
Member Author

At the time of this writing, GitHub is reporting that the Travis build is still waiting, but in fact the tests for this PR ran and passed at https://travis-ci.org/pantheon-systems/terminus/builds/426962621

@greg-1-anderson
Copy link
Member Author

Merged as 2fb8833

@bob-hinrichs
Copy link

bob-hinrichs commented Sep 14, 2018

Figured out that in config.yml, instead of setting this at the command:remote:drush:options level, this works by just putting it at top level.

@TeslaDethray TeslaDethray deleted the git-mode-warning branch February 20, 2019 21:34
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.

None yet

3 participants