@geekdenz opened this Pull Request on September 15th 2021 Contributor

Description:

fixes #17589

Review

@geekdenz commented on September 20th 2021 Contributor

The SEO test fails consistently here, but this seems to have nothing to do with this PR.

@sgiehl commented on September 21st 2021 Member

Just out of curiosity: Why do we need an option to disable certain diagnostic checks? If someone wants to remove the private directory checks he can simply achieve that by creating a config\config.php with this content:

return [
    'diagnostics.disabled' => DI\add([
        DI\get(\Piwik\Plugins\Diagnostics\Diagnostic\RequiredPrivateDirectories::class),
        DI\get(\Piwik\Plugins\Diagnostics\Diagnostic\RecommendedPrivateDirectories::class),
    ])
];
@tsteur commented on September 21st 2021 Member

@sgiehl we usually don't ask users to edit config.php if it can be avoided. The ini file is a lot easier to change for people, especially non-technical people. And once they have more entries in config.php it may be easy for them to break things. Also this way it's independent of code. Like recently we changed class names for these and then it wouldn't work anymore or even the entire Matomo could break if we removed these classes maybe as the files would no longer exist.

@geekdenz commented on September 21st 2021 Contributor
@tsteur commented on September 22nd 2021 Member

@geekdenz this looks good to merge.

For some reason thought the PR build always fails see eg https://app.travis-ci.com/github/matomo-org/matomo/jobs/539047123#L1189

and also in the previous run see https://app.travis-ci.com/github/matomo-org/matomo/builds/238279592

Not sure if there went something wrong with .gitmodules etc. It says:

image

@bx80 experienced this recently as well and fixed it with https://developer.matomo.org/guides/git#fixing-the-error-fatal-remote-error-upload-pack-not-our-ref

This Pull Request was closed on September 26th 2021
Powered by GitHub Issue Mirror