Scrutinizer fixes #317
Scrutinizer fixes #317
Conversation
|
@icewind1991, thanks for your PR! By analyzing the annotation information on this pull request, we identified @jmaciasportela, @rullzer, @schiessle, @DeepDiver1975 and @Xenopathic to be potential reviewers |
| * | ||
| */ | ||
|
|
||
| /* global Handlebars */ |
|
Looks good, but the change in the ShareController looks a bit intrusive |
| private $baseUri; | ||
|
|
||
| /** @var Connector\Sabre\Server */ | ||
| private $server; |
There was a problem hiding this comment.
Needs to be protected
PHP Fatal error: Cannot access private property OCA\DAV\Server::$server in /drone/src/github.com/nextcloud/server/apps/dav/tests/unit/ServerTest.php
icewind1991
force-pushed
the
scrutinizer-fixed
branch
from
98be1b2 to
1c5d8ed
Compare
|
@nickvergessen all fixed |
But still CI is not happy: |
|
Okay maybe you need to use invokePrivate in the test, when you don't want to make it public. |
Or for now simply drop that single change to have only the easier changes in here and for the specific one, that also touches tests you could open a separate PR ;) |
|
Changed the test, the assert that used the property was unneeded |
icewind1991
added
3. to review
LukasReschke
force-pushed
the
scrutinizer-fixed
branch
from
37fbf38 to
d0e6fdb
Compare
|
👍 |
1 similar comment
|
👍 |
…xtcloud#317) This PR adds missing check for TopMenu entry registered only for admins. Signed-off-by: Andrey Borysenko <[email protected]>
Lets try making scrutinizer a bit happier
Contains one or two things which might have actually caused issues previously