-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Ocm to groups 20230717 #40877
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
Ocm to groups 20230717 #40877
Conversation
Signed-off-by: Michiel de Jong <[email protected]>
All Broken tests are fixed Signed-off-by: Michiel de Jong <[email protected]> Signed-off-by: navid-shokri <[email protected]>
| return null; | ||
| } | ||
|
|
||
| private function getManagerForShareType($share_type) { |
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 think we should change this variable name, and other related ones.
I thought this was referring to the share constants (from the "lib/private/Share/Constants.php" file) but it doesn't seem to be the case. That's why I suggested to use a constant for the comparison below.
|
https://drone.owncloud.com/owncloud/core/38747/58/14 Quite a lot of test scenarios are failing. I was hoping that I could just cherry-pick the relevant code from #40589 and have a "clean" PR with green CI. But it seems not to be. @michielbdejong @navid-shokri @soltanireza65 and/or whoever is getting this code going - maybe you can clean up your original PR and get it passing again? |
|
@phil-davis close in favour of #40886 (which is getting green CI) ? |
|
Yes, that has cleaned-up code with green CI. |
Description
I have cherry-picked the real commits from the 221 commits currently in PR #40589
I think that this PR has the real code changes, with all the "merge to/from master" stuff that has got in there somehow.
Maybe this PR will be easier to review.
Related Issue
How Has This Been Tested?
CI
Types of changes
Checklist: