-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
move files_sharing to IBootStrap #28314
Conversation
@icewind1991 can you do a bit more thorough testing ? this is especially important when refactoring such deep bits of code |
Sorry, I didn't notice because the "missing image" in the navbar just shows as transparent for me, so unless I hover over the topbar the extra nav item is invisible. Fixed. |
53c4e50
to
373b0ef
Compare
This error doesn't appear on master. |
forgot that external sharing is semi-split between 2 apps, fixed now (and improved my dev setup to allow for easier federated sharing testing) |
indeed. my trick is to always pick a convoluted test scenario that is connected to the topic and is likely to break 😉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested a few scenarios like federated shares, federated group shares, link share, group share and user share, all seemed fine now 👍
Can you also add the missing return types in the code parts you have touched ? (see code scanner comments in the PR diff)
Is failing. php:cs too :) |
@icewind1991 please fix with cs:fix |
🏓 |
CI is failing |
Seems unrelated |
this is needed though |
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
ed1ccb5
to
033a83b
Compare
reduce debug log spam