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/move image to oc namespace #47535

Merged
merged 6 commits into from
Aug 29, 2024
Merged

Fix/move image to oc namespace #47535

merged 6 commits into from
Aug 29, 2024

Conversation

come-nc
Copy link
Contributor

@come-nc come-nc commented Aug 27, 2024

Summary

Move legacy OC_Image into \OC\Image and update code style and remove deprecated references in there.
This class is exposed through OCP\Image which is used by applications with new \OCP\Image() and then \OCP\IImage methods can be used on it.

This still leaves a reference to OC from OCP which we try to avoid.
But this would mean adding an IImageFactory which applications needs to DI, not sure this is worth it.

Checklist

@come-nc come-nc added 3. to review Waiting for reviews technical debt labels Aug 27, 2024
@come-nc come-nc added this to the Nextcloud 31 milestone Aug 27, 2024
@come-nc come-nc self-assigned this Aug 27, 2024
@come-nc come-nc force-pushed the fix/move-image-to-oc-namespace branch from 17dec99 to 0010f28 Compare August 27, 2024 15:35
@come-nc come-nc requested review from artonge, a team, icewind1991 and sorbaugh and removed request for a team August 27, 2024 15:57
lib/public/Image.php Outdated Show resolved Hide resolved
@come-nc come-nc force-pushed the fix/move-image-to-oc-namespace branch from f296d19 to 1f46be7 Compare August 29, 2024 15:06
@come-nc come-nc merged commit e44f24f into master Aug 29, 2024
174 checks passed
@come-nc come-nc deleted the fix/move-image-to-oc-namespace branch August 29, 2024 16:01
@provokateurin
Copy link
Member

provokateurin commented Sep 2, 2024

This broke Talk nextcloud/spreed#13197 and using the OCP replacement does not work.
I think the problem is the \OCP\Image depends on \OC\Image which does not exist in the dependency and thus all methods are missing.
Also some of the methods are only in \OC\Image and not in \OCP\Image or \OCP\IImage and thus are not visible.

To fix this \OCP\Image needs to implement \OCP\IImage directly and the missing method need to be added to \OCP\IImage.

@come-nc come-nc added the pending documentation This pull request needs an associated documentation update label Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews pending documentation This pull request needs an associated documentation update technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants