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

Fix CI issues and workflows #53

Merged
merged 15 commits into from
Aug 23, 2024
Merged

Conversation

thlehmann-ionos
Copy link
Collaborator

@thlehmann-ionos thlehmann-ionos commented Aug 20, 2024

Biggest problems still:

We reference private/internal Nextcloud types from the OC\ namespace, which are officially not visible - with this PR we add ignores for them

@thlehmann-ionos thlehmann-ionos force-pushed the tl/dev/fix-ci-issues-and-workflows branch from b7e7dd4 to c4ad33b Compare August 20, 2024 12:53
@thlehmann-ionos thlehmann-ionos marked this pull request as ready for review August 20, 2024 15:37
@thlehmann-ionos thlehmann-ionos force-pushed the tl/dev/fix-ci-issues-and-workflows branch 2 times, most recently from 003049d to 7c8b392 Compare August 21, 2024 07:49
Despite the current RC being 30.x tests enabling the app on
master fail with version 30.
I opted for tabs because Nextcloud use mostly tabs in their psalm.xml too.
* Use the same error level as Nextcloud core
* Reduce initial number of issues from ~70 to ~20
Just for food measure, doesn't seem to have an effect on detected
issues.
UndefinedClass:

   Since we're referencing the private implementation Psalm can't
   "see" the interface.
Psalm complains

   Method (OCP\Authentication\Token\IToken&OC\Authentication\Token\INamedToken)::setName does not exist

due to findTokenByIdAndUser() returning IToken, which is too closed and
does not contain setName() declared by INamedToken.

Extending the type range allows Psalm to "see" setName()
To ignore issue that this class is not used.
As they only create unfixable Psalm noise.
This causes Psalm issues because it can not find IAuthTokenProvider.

Apparently it's not able to know that this type was imported as another
name.
Otherwise Psalm would complain

   OC\Authentication\Token\IToken
   does not contain
   OC\Authentication\Token\INamedToken

which is understandable.
@tanyaka
Copy link
Contributor

tanyaka commented Aug 21, 2024

Would propose to fix here also deprecations warning for unit tests in apps-custom/simplesettings/tests/Controller/AuthSettingsControllerTest.php:296 and apps-custom/simplesettings/tests/Controller/AuthSettingsControllerTest.php:253 so unit tests will get green.

@tanyaka tanyaka self-requested a review August 23, 2024 08:56
@tanyaka tanyaka merged commit 796ab3d into master Aug 23, 2024
29 checks passed
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.

2 participants