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

[site:statistics] command fails when comment module is uninstalled. Fix #2744 #2799

Merged
merged 2 commits into from
Oct 15, 2016

Conversation

daydiff
Copy link
Contributor

@daydiff daydiff commented Oct 12, 2016

fix failing command [site:statistics] when comment module is uninstalled

try {
$entityQuery = $this->entityQuery->get('comment')->count();
} catch (PluginNotFoundException $e) {
return 0;
Copy link
Member

Choose a reason for hiding this comment

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

@daydiff Thanks for the PR

if any error the CLI application should a value greater than 0 updating to 1 will do the work here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But it is not the value CLI application returns, but the value of comments which is 0 when "Comment" module is not installed. Btw I agree, that it would be better to not catch exception but prevent it. I can replace catch block with something like this:

if (!\Drupal::moduleHandler()->moduleExists('comment')) {
    return 0;
}

What do you think about such changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I replaced try-catch block so it is no exception now.

Copy link
Member

Choose a reason for hiding this comment

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

My bad, but like this better value 0 instead of PluginNotFoundException

@jmolivas jmolivas changed the title Fix #2744 [site:statistics] command fails when comment module is uninstalled. Fix #2744 Oct 13, 2016
@jmolivas jmolivas merged commit 5f9aa6a into hechoendrupal:master Oct 15, 2016
@jmolivas jmolivas added this to the 1.0.0-rc6 milestone Oct 15, 2016
@jmolivas
Copy link
Member

@daydiff Thanks for your contribution, your PR was merged. This will be included on the next release.

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.

2 participants