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.

Sign up for GitHub

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

Jump to bottom

implement phpstan #39

Merged
merged 9 commits into from
Feb 16, 2022
Merged

implement phpstan #39

merged 9 commits into from
Feb 16, 2022

Conversation

individual-it
Copy link
Collaborator

@individual-it individual-it commented Feb 4, 2022 •
edited
Loading

  • run PHPstan in CI
  • fix issues phpstan complained about
  • run tests with NC 23 and NC dev version (master branch)

@individual-it
Copy link
Collaborator Author

@eneiluj phpstan found 3 errors I would like to discuss with you

------ -------------------------------------------------------------------- 
  Line   lib/Listener/LoadSidebarScript.php                                  
 ------ -------------------------------------------------------------------- 
  40     Static method OCP\Util::addScript() invoked with 3 parameters, 1-2  
         required.                                                           
 ------ --------------------------------------------------------------------

This was changed after v.23 see nextcloud/server#30015
so should we increase the min-version of server for the app to 24?

 ------ --------------------------------------------------------------------- 
  Line   lib/Search/OpenProjectSearchProvider.php                             
 ------ --------------------------------------------------------------------- 
  190    Method OCA\OpenProject\Search\OpenProjectSearchProvider::truncate()  
         is unused.                                                           
 ------ --------------------------------------------------------------------- 

 ------ ----------------------------------------------------------------------- 
  Line   lib/Service/OpenProjectAPIService.php                                  
 ------ ----------------------------------------------------------------------- 
  145    Method OCA\OpenProject\Service\OpenProjectAPIService::getWPStatusId()  
         is unused.                                                             
 ------ -----------------------------------------------------------------------

delete those functions, or what are they for?

individual-it
individual-it commented Feb 7, 2022
View reviewed changes
lib/Controller/OpenProjectAPIController.php
@@ -99,6 +99,8 @@ public function getOpenProjectAvatar(string $userId = '', string $userName = '')
$this->openprojectUrl, $this->accessToken, $this->tokenType, $this->refreshToken,
$this->clientID, $this->clientSecret, $userId, $userName
);
// TODO this will never happen, because `getOpenProjectAvatar` does not return ever an error result
// @phpstan-ignore-next-line
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

issue created, see https://community.openproject.org/projects/nextcloud-integration/work_packages/details/40968/overview

@individual-it
run phpstan in CI
e2d639f 
Signed-off-by: Artur Neumann <artur@jankaritech.com>
@individual-it
fix phpstan issues
2e38e21 
Signed-off-by: Artur Neumann <artur@jankaritech.com>
@individual-it
run tests with NC 23 & master
9ee0754 
Signed-off-by: Artur Neumann <artur@jankaritech.com>
@individual-it
add script differently depending on the version of nextcloud
b93e327 
Signed-off-by: Artur Neumann <artur@jankaritech.com>
@individual-it
delete unused methods
c84ad88 
Signed-off-by: Artur Neumann <artur@jankaritech.com>
@individual-it
fix php-cs-fixer
147e4e7 
Signed-off-by: Artur Neumann <artur@jankaritech.com>
@individual-it
load OC for unit tests
c1c6268 
Signed-off-by: Artur Neumann <artur@jankaritech.com>
@individual-it
install NC before running tests
ff43b1d 
Signed-off-by: Artur Neumann <artur@jankaritech.com>
@individual-it
coverage report only when running with NC master
6cb56e0 
Signed-off-by: Artur Neumann <artur@jankaritech.com>
@github-actions
Copy link

JS Code Coverage

Coverage after merging phpstan into master will be
24.34%
Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src
   utils.js8.33%0%0%14.29%3–9, 9
src/components
   AdminSettings.vue0%100%0%0%101, 48–54, 66, 82–84, 88, 95–96, 98
   PersonalSettings.vue24.76%15.15%26.32%30.19%126, 126, 126, 126, 126, 137–140, 146–147, 150–151, 151, 151, 154–155, 155, 155, 158–159, 159, 159, 162–164, 164, 164, 164–165, 167–168, 168, 168–169, 172, 172, 175, 178, 178, 184, 187–188, 190–191, 191–193, 193, 193, 193–194, 199–200, 200, 200, 205, 211, 215–216, 222, 228–229, 231, 234
src/components/settings
   SettingsTitle.vue0%100%0%0%14
src/components/tab
   EmptyContent.vue100%100%100%100%
src/views
   Dashboard.vue0%0%0%0%100, 106, 106, 106–107, 109, 115, 119–120, 128, 131, 135–137, 139, 142–143, 146–147, 147–148, 152–153, 153–154, 156, 158–159, 159, 159, 159, 159–161, 161, 161, 161, 161–163, 166, 171, 171, 171, 173–174, 174, 174–175, 177, 177–179, 183, 187, 190, 190, 190, 193, 193, 193, 198, 201, 201, 201, 208, 211, 211, 211, 216, 216, 216, 221, 221, 221, 226, 229, 232, 235, 235, 235, 238, 238, 238, 24, 241, 244, 247, 25–30, 47, 53, 53, 60, 63–64, 76–77, 77, 77, 80, 83, 83, 83–85, 85, 85–87, 87–88, 90, 93, 93, 93–95, 95, 95–97, 97–98
   ProjectsTab.vue90.32%100%83.33%89.47%66–67

@github-actions
Copy link

PHP Code Coverage

Coverage after merging phpstan into master will be
36.03%
Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
lib/AppInfo
   Application.php0%100%0%0%101, 105, 49, 51–52, 54–56, 59–60, 63–65, 67, 70–71, 74–77, 79–84, 86, 88, 91, 95, 99
lib/BackgroundJob
   CheckNotifications.php0%100%0%0%48, 50, 52–53, 61–62
lib/Controller
   ConfigController.php66.67%100%60%67.07%106–107, 109, 184, 194–197, 199, 201, 71–72, 74, 76–78, 80–87, 92–93, 95
   OpenProjectAPIController.php64.52%100%50%66.67%100, 104–105, 107–108, 110, 82, 98–99
lib/Dashboard
   OpenProjectWidget.php0%100%0%0%44–45, 52, 59, 66, 73, 80, 87–88
lib/Listener
   LoadSidebarScript.php0%100%0%0%37–38, 40, 42, 44–46, 48
lib/Notification
   Notifier.php0%100%0%0%100, 110, 112, 114, 47–50, 60, 69, 80, 82, 85, 87–92
lib/Search
   OpenProjectSearchProvider.php0%100%0%0%102, 109–110, 113–116, 118–119, 123, 127–132, 134–136, 139–140, 142–143, 147–148, 158, 170, 178, 181, 184, 193, 196, 207, 71–75, 82, 89, 97, 99
   OpenProjectSearchResultEntry.php100%100%100%100%
lib/Service
   OpenProjectAPIService.php47.57%100%50%47.42%104–114, 116–120, 122–127, 129–130, 133–136, 143, 150–154, 164–165, 167–171, 173–174, 182–186, 306–310, 312–313, 317–318, 320–321, 323–327, 351, 366–367, 369, 375, 381–386, 388, 394, 422–425, 428–429, 431, 433–434, 456–457, 464, 467–470, 472, 478, 482–484, 94–97
lib/Settings
   Admin.php0%100%0%0%25–26, 33–35, 37, 42–43, 47, 51
   AdminSection.php0%100%0%0%19–20, 29, 39, 48, 55
   Personal.php0%100%0%0%31–33, 40–46, 49, 51–52, 54, 66–67, 71, 75
   PersonalSection.php0%100%0%0%19–20, 29, 39, 48, 55

@individual-it individual-it marked this pull request as ready for review February 15, 2022 10:53
@individual-it individual-it self-assigned this Feb 15, 2022
kiranparajuli589
kiranparajuli589 approved these changes Feb 15, 2022
View reviewed changes
Copy link
Contributor

@kiranparajuli589 kiranparajuli589 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@individual-it individual-it merged commit ca51dba into master Feb 16, 2022
@delete-merged-branch delete-merged-branch bot deleted the phpstan branch February 16, 2022 03:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kiranparajuli589 kiranparajuli589 kiranparajuli589 approved these changes

@julien-nc julien-nc Awaiting requested review from julien-nc

Assignees

@individual-it individual-it

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@individual-it @kiranparajuli589