Skip to content

Conversation

ChristophWurst
Copy link
Member

Resolves: n/a

Summary

PHP allows us to add types to a base class that is extended elsewhere without breaking an interface: https://3v4l.org/JDX4H. So let's make use of this.

Checklist

@nickvergessen
Copy link
Member

But Psalm might cry?

@ChristophWurst
Copy link
Member Author

But Psalm might cry?

Always has

@ChristophWurst
Copy link
Member Author

But Psalm might cry?

Only INFO, no ERROR: https://psalm.dev/r/ccd19c3340

@juliusknorr juliusknorr added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jan 25, 2023
@ChristophWurst
Copy link
Member Author

tests fail because we pass null as method name

@ChristophWurst ChristophWurst added 2. developing Work in progress and removed 4. to release Ready to be released and/or waiting for tests to finish labels Jan 25, 2023
@ChristophWurst ChristophWurst marked this pull request as draft January 25, 2023 18:37
@blizzz blizzz mentioned this pull request Feb 1, 2023
@skjnldsv skjnldsv mentioned this pull request Feb 23, 2023
@blizzz blizzz mentioned this pull request Mar 7, 2023
@blizzz blizzz modified the milestones: Nextcloud 26, Nextcloud 27 Mar 9, 2023
@szaimen
Copy link
Contributor

szaimen commented Apr 17, 2023

What is missing here?

@ChristophWurst
Copy link
Member Author

tests fail because we pass null as method name

Test\AppFramework\Middleware\MiddlewareTest::testBeforeController
--


TypeError: 
OCP\AppFramework\Middleware::beforeController(): Argument #2 
($methodName) must be of type string, null given, called in 
/drone/src/tests/lib/AppFramework/Middleware/MiddlewareTest.php on line 
74

@nickvergessen
Copy link
Member

nickvergessen commented Apr 18, 2023

So?

  • Fix up
  • Mark as ready to review
  • Merge

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@ChristophWurst ChristophWurst force-pushed the feat/app-framework/middleware-argument-types branch from 1e4a519 to 2c0cfd3 Compare April 18, 2023 15:16
@ChristophWurst ChristophWurst marked this pull request as ready for review April 18, 2023 15:17
@ChristophWurst ChristophWurst added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Apr 18, 2023
@nickvergessen nickvergessen enabled auto-merge April 18, 2023 15:35
@nickvergessen nickvergessen merged commit bfaac51 into master Apr 18, 2023
@nickvergessen nickvergessen deleted the feat/app-framework/middleware-argument-types branch April 18, 2023 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Development

Successfully merging this pull request may close these issues.

5 participants