loopline-systems / closeio-api-wrapper

PHP Wrapper to use the Close.io API
MIT License
14 stars 15 forks source link

Docblock Improvements + fix for ActivityApi::addNote Result #35

Closed wodka closed 6 years ago

wodka commented 6 years ago
mavimo commented 6 years ago

@wodka can you rebase it, please?

wodka commented 6 years ago

I will do that - might just take a few weeks till I have time

mavimo commented 6 years ago

@wodka ok, thanks, let me know when it's done.

mavimo commented 6 years ago

@wodka do you think to be able to rebase this week? let me know in order to include it on release 0.7 or in the next release.

wodka commented 6 years ago

@mavimo sry for the delay - just did the rebase

coveralls commented 6 years ago

Coverage Status

Coverage increased (+7.7%) to 33.443% when pulling ccb2bee1ee51062e49c8b1a3f748758c44283038 on wodka:minor-fixes into badc0a645c83dad3a09c2b592b3effd211dab5f6 on loopline-systems:develop.

wodka commented 6 years ago

I do not get why it is failing on php71/72 :/

mavimo commented 6 years ago

@wodka with PHP7 we run also phpstan for static analysis, this tool actually report that the annotation reported do not match with the current code. The reported errors are:


 ------ ------------------------------------------------------------------------- 
  Line   src/LooplineSystems/CloseIoApiWrapper/CloseIoApiWrapper.php              
 ------ ------------------------------------------------------------------------- 
  72     Method                                                                   
         LooplineSystems\CloseIoApiWrapper\CloseIoApiWrapper::getLeadApi()        
         should return LooplineSystems\CloseIoApiWrapper\Library\Api\AbstractApi  
         but returns null.                                                        
  84     Method                                                                   
         LooplineSystems\CloseIoApiWrapper\CloseIoApiWrapper::getCustomFieldApi(  
         ) should return                                                          
         LooplineSystems\CloseIoApiWrapper\Library\Api\AbstractApi but returns    
         null.                                                                    
  96     Method                                                                   
         LooplineSystems\CloseIoApiWrapper\CloseIoApiWrapper::getOpportunityApi(  
         ) should return                                                          
         LooplineSystems\CloseIoApiWrapper\Library\Api\AbstractApi but returns    
         null.                                                                    
  108    Method                                                                   
         LooplineSystems\CloseIoApiWrapper\CloseIoApiWrapper::getLeadStatusesApi  
         () should return                                                         
         LooplineSystems\CloseIoApiWrapper\Library\Api\AbstractApi but returns    
         null.                                                                    
  120    Method                                                                   
         LooplineSystems\CloseIoApiWrapper\CloseIoApiWrapper::getOpportunityStat  
         usesApi() should return                                                  
         LooplineSystems\CloseIoApiWrapper\Library\Api\AbstractApi but returns    
         null.                                                                    
  132    Method                                                                   
         LooplineSystems\CloseIoApiWrapper\CloseIoApiWrapper::getContactApi()     
         should return LooplineSystems\CloseIoApiWrapper\Library\Api\AbstractApi  
         but returns null.                                                        
  144    Method                                                                   
         LooplineSystems\CloseIoApiWrapper\CloseIoApiWrapper::getActivitiesApi()  
         should return LooplineSystems\CloseIoApiWrapper\Library\Api\AbstractApi  
         but returns null.                                                        
  156    Method                                                                   
         LooplineSystems\CloseIoApiWrapper\CloseIoApiWrapper::getTaskApi()        
         should return LooplineSystems\CloseIoApiWrapper\Library\Api\AbstractApi  
         but returns null.                                                        
 ------ ------------------------------------------------------------------------- 
 ------ ----------------------------------------------------------------------- 
  Line   tests/LooplineSystems/CloseIoApiWrapper/Library/Curl/CurlTest.php      
 ------ ----------------------------------------------------------------------- 
  47     Call to an undefined method                                            
         LooplineSystems\CloseIoApiWrapper\Library\Api\AbstractApi::getLead().  
 ------ ----------------------------------------------------------------------- 
 ------ ------------------------------------------------------------------------- 
  Line   tests/LooplineSystems/CloseIoApiWrapper/Api/CustomFieldApiTest.php       
 ------ ------------------------------------------------------------------------- 
  94     Method                                                                   
         Tests\LooplineSystems\CloseIoApiWrapper\Api\CustomFieldApiTest::getCust  
         omFieldApi() should return                                               
         LooplineSystems\CloseIoApiWrapper\Api\CustomFieldApi but returns         
         LooplineSystems\CloseIoApiWrapper\Library\Api\AbstractApi.               
 ------ ------------------------------------------------------------------------- 
 ------ ------------------------------------------------------------------------- 
  Line   tests/LooplineSystems/CloseIoApiWrapper/Api/ActivityApiTest.php          
 ------ ------------------------------------------------------------------------- 
  77     Method                                                                   
         Tests\LooplineSystems\CloseIoApiWrapper\Api\ActivityApiTest::getActivit  
         yApi() should return LooplineSystems\CloseIoApiWrapper\Api\ActivityApi   
         but returns LooplineSystems\CloseIoApiWrapper\Library\Api\AbstractApi.   
 ------ ------------------------------------------------------------------------- 
 ------ ------------------------------------------------------------------------- 
  Line   tests/LooplineSystems/CloseIoApiWrapper/Api/LeadsApiTest.php             
 ------ ------------------------------------------------------------------------- 
  171    Method                                                                   
         Tests\LooplineSystems\CloseIoApiWrapper\Api\LeadsApiTest::getLeadsApi()  
         should return LooplineSystems\CloseIoApiWrapper\Api\LeadApi but returns  
         LooplineSystems\CloseIoApiWrapper\Library\Api\AbstractApi.               
 ------ ------------------------------------------------------------------------- 
 ------ ------------------------------------------------------------------------ 
  Line   tests/LooplineSystems/CloseIoApi/Api/ConfigTest.php                     
 ------ ------------------------------------------------------------------------ 
         Class LooplineSystems\CloseIoApiWrapper\Tests\ConfigTest was not found  
         while trying to analyse it - autoloading is probably not configured     
         properly.                                                               
 ------ ------------------------------------------------------------------------ 

 [ERROR] Found 13 errors                                                        

for example I see you added:

    /**
     * @return LeadApi|AbstractApi
     */
    public function getLeadApi()
        try {
            return $this->apiHandler->getApi(LeadApi::NAME);
        } catch (ApiNotFoundException $e) {
            return null;
        }
    }

The annotation indicate that the method getLeadApi should return an LeadApi or an AbstractApi but in the code path we also return null that is not a LeadApi or AbstractApi; Maybe we can indicate that return a LeadApi or null; I suggest to use

    /**
     * @return ?LeadApi
     */
    public function getLeadApi() {}

as annotated format (? mean nullable in PHP 7.1)