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

[stable25] Backport: Make the container fully fulfill PSR container interface #36618

Merged
merged 3 commits into from
May 12, 2023
Merged

[stable25] Backport: Make the container fully fulfill PSR container interface #36618

merged 3 commits into from
May 12, 2023

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Feb 8, 2023

Checklist

@susnux susnux requested review from nickvergessen, ChristophWurst, a team, ArtificialOwl, icewind1991 and blizzz and removed request for a team February 8, 2023 16:28
@susnux
Copy link
Contributor Author

susnux commented Feb 8, 2023

To fix the psalm errors we would have to either update the psr container to 1.1.2.
But somehow it does not complain about the @throws ContainerExceptionInterface which was there before and did not extend Throwable either.

@blizzz blizzz mentioned this pull request Feb 9, 2023
Merged
@susnux susnux added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Feb 9, 2023
github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 9, 2023
View reviewed changes
lib/public/IContainer.php
@@ -71,6 +72,7 @@
* @return mixed
* @psalm-return ($name is class-string ? T : mixed)
* @throws ContainerExceptionInterface if the query could not be resolved
* @throws NotFoundExceptionInterface if the name could not be found within the container

Check failure

Code scanning / Psalm

InvalidThrow

Class supplied for @throws Psr\Container\NotFoundExceptionInterface does not implement Throwable
@blizzz
Copy link
Member

blizzz commented Feb 9, 2023

CI unhappy, moving to 25.0.5

@susnux susnux force-pushed the backport/36417 branch 3 times, most recently from b77f72e to 799cf93 Compare February 20, 2023 12:23
@susnux susnux mentioned this pull request Feb 20, 2023
Merged
@susnux susnux force-pushed the backport/36417 branch 2 times, most recently from f1d6e11 to 84a2dc7 Compare February 20, 2023 17:35
@blizzz
Copy link
Member

blizzz commented Mar 15, 2023

/rebase

@susnux susnux force-pushed the backport/36417 branch 2 times, most recently from 2111a59 to 309b2cc Compare March 15, 2023 13:49
@susnux
Copy link
Contributor Author

susnux commented Mar 15, 2023

/rebase

@blizzz Did not work, so I rebased it manually.

@susnux susnux requested a review from come-nc March 15, 2023 17:32
@come-nc
Copy link
Contributor

come-nc commented Mar 16, 2023

I do not understand what the hell is happenning to psalm here. It’s removing stuff from the baseline and then complaining about it?

@susnux
Copy link
Contributor Author

susnux commented Mar 16, 2023 •
edited
Loading

I do not understand what the hell is happenning to psalm here. It’s removing stuff from the baseline and then complaining about it?

@come-nc I resolved this. It happened because updating the psr container also triggered updating psalm from 4.18.1 to 4.30.0. This caused psalm to report new (?) errors. So I reverted the update of psalm.

But I still have no clue why cs fixer is failing, the reported files are unchanged (even if cs fixer is right about the wrong code style within those files)... 👈 fixed

@nickvergessen
Copy link
Member

❌ Branch is currently frozen

@skjnldsv skjnldsv mentioned this pull request Apr 13, 2023
Merged
11 tasks
@susnux
Copy link
Contributor Author

susnux commented Apr 26, 2023

@nickvergessen branch is no longer frozen

drone failure is unrelated

@susnux susnux force-pushed the backport/36417 branch 3 times, most recently from 82cb2a5 to 9653247 Compare May 3, 2023 13:44
@blizzz
Copy link
Member

blizzz commented May 9, 2023

just merged 3rdparty PR. Rebase needed?

susnux added 3 commits May 11, 2023 13:03
@susnux
chore: Update 3rdparty to update psr/container to 1.1.2
21809e3 
Signed-off-by: Ferdinand Thiessen <rpm@fthiessen.de>
@susnux
fix: Throw NotFoundExceptionInterface to fulfill PSR container inte…
323afd1 
…rface if class not found

Signed-off-by: Ferdinand Thiessen <rpm@fthiessen.de>
@susnux
Update autoloader
f833319 
Signed-off-by: Ferdinand Thiessen <rpm@fthiessen.de>
@susnux
Copy link
Contributor Author

susnux commented May 11, 2023 •
edited
Loading

just merged 3rdparty PR. Rebase needed?

@blizzz Done

@blizzz blizzz merged commit 4c32edf into stable25 May 12, 2023
@blizzz blizzz deleted the backport/36417 branch May 12, 2023 14:32
@blizzz blizzz mentioned this pull request May 16, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nickvergessen nickvergessen nickvergessen approved these changes

@ChristophWurst ChristophWurst ChristophWurst approved these changes

@come-nc come-nc come-nc approved these changes

@ArtificialOwl ArtificialOwl Awaiting requested review from ArtificialOwl ArtificialOwl was automatically assigned from nextcloud/server-backend

@icewind1991 icewind1991 Awaiting requested review from icewind1991 icewind1991 was automatically assigned from nextcloud/server-backend

@blizzz blizzz Awaiting requested review from blizzz blizzz was automatically assigned from nextcloud/server-backend

Assignees
No one assigned
Labels
4. to release Ready to be released and/or waiting for tests to finish bug integration
Projects
None yet
Milestone
Nextcloud 25.0.7
Development

Successfully merging this pull request may close these issues.
7 participants
@susnux @blizzz @come-nc @nickvergessen @ChristophWurst @skjnldsv @szaimen