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

Add phpstan to travis #1091

Merged
merged 1 commit into from Oct 19, 2018
Merged

Add phpstan to travis #1091

merged 1 commit into from Oct 19, 2018

Conversation

DeepDiver1975
Copy link
Member

No description provided.

@DeepDiver1975 DeepDiver1975 force-pushed the feature/add-phpstan branch 5 times, most recently from 5759083 to 4c94f06 Compare October 5, 2018 17:29
@codecov
Copy link

codecov bot commented Oct 5, 2018

Codecov Report

Merging #1091 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1091      +/-   ##
============================================
- Coverage     97.21%   97.19%   -0.02%     
+ Complexity     2871     2865       -6     
============================================
  Files           174      174              
  Lines          8067     8057      -10     
============================================
- Hits           7842     7831      -11     
- Misses          225      226       +1
Impacted Files Coverage Δ Complexity Δ
lib/DAV/Browser/GuessContentType.php 100% <ø> (ø) 4 <0> (ø) ⬇️
lib/CalDAV/Plugin.php 97.98% <ø> (ø) 121 <0> (ø) ⬇️
lib/CalDAV/Xml/Request/FreeBusyQueryReport.php 94.73% <ø> (ø) 9 <0> (ø) ⬇️
lib/DAVACL/Xml/Request/ExpandPropertyReport.php 94.73% <ø> (ø) 6 <0> (ø) ⬇️
lib/CalDAV/Xml/Notification/InviteReply.php 100% <100%> (ø) 12 <0> (ø) ⬇️
lib/CalDAV/SharingPlugin.php 98.41% <100%> (-0.12%) 40 <0> (-6)
lib/DAV/Exception/Locked.php 100% <100%> (ø) 4 <0> (ø) ⬇️
lib/DAV/Xml/Property/Href.php 100% <100%> (ø) 12 <0> (ø) ⬇️
lib/CalDAV/Xml/Notification/Invite.php 94.82% <100%> (ø) 21 <0> (ø) ⬇️
lib/DAV/Exception/LockTokenMatchesRequestUri.php 100% <100%> (ø) 2 <0> (ø) ⬇️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4d8d0a4...bdbb2a6. Read the comment docs.

@@ -3,6 +3,7 @@
namespace Sabre\CalDAV;

use DateTimeZone;
use Sabre\CalDAV\Xml\Request\CalendarMultiGetReport;
Copy link
Member

Choose a reason for hiding this comment

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

Hmm is it really missing?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is used in PHPDoc as type annotation

@@ -136,8 +136,7 @@ function propFindEarly(DAV\PropFind $propFind, DAV\INode $node) {
}

return new Xml\Property\Invite(
$node->getInvites(),
$ownerInfo
$node->getInvites()
Copy link
Member

Choose a reason for hiding this comment

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

When ownerInfo really is not required here, we should also delete some of the logic above this line

Copy link
Member Author

Choose a reason for hiding this comment

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

Invite:__construct only take one arg .... I will review the code once more .... THX for the pointer

phpstan.neon Outdated
bootstrap: %currentWorkingDirectory%/vendor/autoload.php

excludes_analyse:
- %currentWorkingDirectory%/lib/DAV/Auth/Backend/IMAP.php
Copy link
Member

Choose a reason for hiding this comment

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

Why excluded?

Copy link
Member Author

Choose a reason for hiding this comment

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

Will add comment - because the imap module is only a suggested dependency .... need to have a second look ...

@@ -69,7 +69,7 @@ static function xmlDeserialize(Reader $reader) {
* {DAV:}property elements.
*
* @param array $elems
* @return void
* @return array
Copy link
Member

Choose a reason for hiding this comment

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

Hmm string[] ?

Copy link
Member Author

Choose a reason for hiding this comment

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

the returned array could also hold an array of strings itself .... just given by the code logic

Copy link
Member

@staabm staabm left a comment

Choose a reason for hiding this comment

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

Thx for doing all this cool stuff!

@DeepDiver1975 DeepDiver1975 force-pushed the feature/add-phpstan branch 3 times, most recently from 16f6c8c to b80b543 Compare October 5, 2018 20:55
@DeepDiver1975
Copy link
Member Author

@staabm final review please - all should be sorted out now. THX

@@ -46,6 +46,7 @@ class SchedulingObject extends \Sabre\CalDAV\CalendarObject implements IScheduli
* @param array $objectData
*/
function __construct(Backend\SchedulingSupport $caldavBackend, array $objectData) {
parent::__construct($caldavBackend, [], $objectData);

$this->caldavBackend = $caldavBackend;
Copy link
Member

Choose a reason for hiding this comment

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

Is this assignment redundant with the parent impl?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes - I need to have a closer look. The PHPDoc type annotation is different.

@DeepDiver1975
Copy link
Member Author

@staabm ready for some final review? THX

@staabm
Copy link
Member

staabm commented Oct 19, 2018

Hmm seems build is failling

@DeepDiver1975
Copy link
Member Author

@staabm fixed now

@staabm staabm merged commit 0bb48a8 into master Oct 19, 2018
@staabm
Copy link
Member

staabm commented Oct 19, 2018

Cool thx

@staabm staabm deleted the feature/add-phpstan branch October 19, 2018 09:56
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

2 participants