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(routing): Extract routes even with multiple registerRoutes calls #78

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

provokateurin
Copy link
Member

Happens in https://github.com/nextcloud/server/blob/master/apps/user_ldap/appinfo/routes.php, but in this particular case all the newly discovered routes have to be ignored.

@provokateurin
Copy link
Member Author

CI failure is expected...

Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

🙈

Panic instead?

@provokateurin
Copy link
Member Author

It's bad design, but not forbidden 🤷‍♀️

@provokateurin
Copy link
Member Author

I'll wait with merging this one until right before nextcloud/server#42888 is merged so don't constantly have red CI

@nickvergessen
Copy link
Member

It's bad design, but not forbidden 🤷‍♀️

But we can forbid it for OpenApi? 😅

@provokateurin
Copy link
Member Author

I don't see a problem with it tbh. With nextcloud/server#42678 this problem will be solved entirely anyway.

@provokateurin provokateurin added the bug Something isn't working label Jan 17, 2024
@provokateurin provokateurin merged commit 3b7b6ff into main Jan 18, 2024
8 of 10 checks passed
@provokateurin provokateurin deleted the fix/routing/multiple-register-routes branch January 18, 2024 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants