- 
                Notifications
    
You must be signed in to change notification settings  - Fork 9
 
Feature/remove usage of cloudinary context #43
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
base: master
Are you sure you want to change the base?
Feature/remove usage of cloudinary context #43
Conversation
… required instead of optional and removing the usage of cloudinary context
… required instead of optional and removing the usage of cloudinary context
…ng cloudinary context
| 
           @adimiz1 / @tommyg-cld / @michalkcloudinay, can you please review this PR?  | 
    
| 
           Hi @TomerPacific , Thanks for the PR. Our team will review the request and take necessary action accordingly. Thanks,  | 
    
| 
           @adimiz1 - Can you review this PR?  | 
    
| 
           @adimiz1 / @skalahasti-cloudinary - Can anyone review this PR or at least add the hacktoberfest-accepted label?  | 
    
| 
           Hi @TomerPacific , Sorry for the delay. I have sent this over to Adi to look into this further. Thanks,  | 
    
| 
           @skalahasti-cloudinary - Thanks, any chance you can add the hacktoberfest-accepted label?  | 
    
| 
           @adimiz1 / @skalahasti-cloudinary - Four days till hacktoberfest ends. When will you have time to review this? It has been open for more than two weeks.  | 
    
| 
           @adimiz1 - Last chance...  | 
    
| 
           Thank you for your contribution to Hacktoberfest! As a token of our thanks, we at Cloudinary have emailed you with instructions on how to claim your bespoke 2025 swag pack @TomerPacific  | 
    
Brief Summary of Changes
This pull request refactors both the image and video components to require an explicit
Cloudinaryinstance, removing implicit dependency on the globalCloudinaryContext. This change improves code clarity and testability by making dependencies explicit. The test code is also updated to reflect these changes.Dependency Injection Refactor
CldImageWidget(lib/image/cld_image.dart) andCldVideoController(lib/video/cld_video_controller.dart) now require aCloudinaryinstance to be passed explicitly, instead of optionally falling back to a global singleton.cloudinaryin these classes now assume it is non-null and directly reference its properties, removing fallback logic and null checks.Test Updates
test/cld_video_controller_test.dartis updated to create and configure a localCloudinaryinstance instead of using the globalCloudinaryContext, and all test usages ofCldVideoControllerare updated to pass this instance explicitly.Cleanup
cloudinary_context.dartare removed from both the image and video Dart files, as well as from the test file.What does this PR address?
Are tests included?
Reviewer, please note:
Since we don't want to rely on CloudinaryContext internally anymore, as it is deprecated, I made the Cloudinary argument required.
Checklist: