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

oc-imageresizer-plugin is being migrated to core - https://github.com/octobercms/october/pull/5231 #55

Open
lthurston opened this issue Mar 27, 2019 · 28 comments

Comments

@lthurston
Copy link
Contributor

Hi all,
It sounds like @toughdeveloper is no longer able to invest the time necessary to maintain this project (see #53). I'm quite grateful to him for this plugin as my team uses it on most October projects, and see the need for it to live on beyond his maintenance of it.

I'm hoping someone who sees this message would be willing to take on the task of maintaining this plugin for us and for posterity. Unless, of course, there's some other plugin out there that does this better.

I would be willing to do so but I'm afraid I might quickly fall into the same situation that Matthew has, and might not be able to put the time into it that's required. However, I'd be happy to collaborate with a few other maintainers to keep this thing going.

Let's discuss!

Please note that so far I've only referred people to this message who have open PRs on the project. If we need to bring a larger group in, or have the conversation elsewhere (Slack), that's fine by me. I just want to get the ball rolling.

Lucas

@matteotrubini
Copy link
Collaborator

I agree with greetings to @toughdeveloper who did a great job and we also use the plugin in several projects, so we're are interested in actively maintain it.

I suggest as first step to fork the repo - or better, ask to @toughdeveloper if him is open to transfer the ownership or add collaborators to mantain a full project history - to manage existing PR and carry on the code to an up-to-date status.

Finally, I think that we should agree on a 3 to 5 member team to avoid a "single point of failure" on project management.

@matt-pawley
Copy link
Owner

Will be great to see it being maintained again! Happy to add collaborators to the project. Although to avoid the risk of malicious contributions, I would like to remain responsible for the project in October CMS (https://octobercms.com/plugin/toughdeveloper-imageresizer)

@matteotrubini @lthurston - Sound ok?

@chocolata
Copy link
Collaborator

That sounds perfect Matthew. Thank you for your response and your extensive work on this plugin!

@matt-pawley
Copy link
Owner

No worries, invites sent to suggested maintainers.

Just tag me when updates are merged to master. Don't forget to update version.yaml or October store will not update.

@LukeTowers
Copy link
Contributor

Is there interest in having this brought into core?

@chocolata
Copy link
Collaborator

As far as I'm concerned, definitely! I don't start any project without this plugin.

@lthurston
Copy link
Contributor Author

Agreed, this is crucial for resizing images from Twig. I'd like to get some of these PRs addressed before we start merging it into the core though.

@matteotrubini
Copy link
Collaborator

It's a must-have, like https://github.com/alserom/octobercms_clearcachewidget

@matt-pawley
Copy link
Owner

Including in core would be ideal IMO.

@LukeTowers
Copy link
Contributor

Can I have a point form list of each individual feature that this plugin adds to plan how to best integrate it with the core?

@lthurston
Copy link
Contributor Author

Summarizing README.md:

  • Adds twig filter for generating thumbs based on path to image (including resize, crop, quality, and file format)
  • Integrates with tinypng for image compression (I haven't used this, can't vouch for it)
  • Add simple PHP function for resizing images (including resize, crop, etc)
  • Adds thumb generation to backend list configuration (I haven't used this, can't vouch for it)

@matteotrubini
Copy link
Collaborator

@LukeTowers do you have some plans in the short term to integrate the functions or currently it is only in wishlist?

In the meanwhile, I suggest to use reviewers in existing PR to approve them in team - ex: any PR should be approved by the majority of the collaborators to be merged.

@LukeTowers
Copy link
Contributor

@matteotrubini I currently don't have any time to do it myself, but if someone put together a PR I would be more than happy to review it.

@matteotrubini
Copy link
Collaborator

@toughdeveloper I just added a "develop" branch to merge PRs and better manage releases.
Do you agree?

@matt-pawley
Copy link
Owner

@matteotrubini - Sounds good, but avoid any confusion can you clarify the suggested process further please? Are you looking to have release branches which would branch from develop and be merged into master?

@matteotrubini
Copy link
Collaborator

I think Gitflow would be fine: we can manage any merge into master as a PR that need to be approved by reviewers.

@panakour
Copy link
Contributor

panakour commented Jun 14, 2019

@matteotrubini
Copy link
Collaborator

@toughdeveloper if is possibile to change the origin of the plugin in the marketplace, please consider to create a GitHub organization where move the repo and nomenee other admins as it would make tasks like #65 easier

Maintaining the control of marketplace's account, any release will remain in pending status up to your final revision, preventing any malicious code to be distributed, am I correct?

@LukeTowers
Copy link
Contributor

@matteotrubini @toughdeveloper @panakour @lthurston @maartenmachiels FYI: this plugin's functionality will shortly be included in the core by default thanks to some sponsorship from @beckandstone. See octobercms/october#5231.

Please test out that PR (and the related TinyPNG plugin) and provide your feedback! Looking to have it merged in by next Wednesday.

@chocolata
Copy link
Collaborator

Thanks @LukeTowers ! To be sure, is there a chance of sites breaking when both the current plugin is installed side into an October instance with the resize functionality built in? I'd like to prevent situations where clients update their October instance, after which their site would break.

@LukeTowers
Copy link
Contributor

@maartenmachiels nope! If a site is using a plugin that provides the | resize() filter already, then the plugin's definition would override the system definition, so the behaviour would remain unchanged for existing users of the plugin. They would just have to remove the existing plugin and then they'd start getting the benefits of the new functionality in the core which provides better performance and hopefully better stability.

@matteotrubini
Copy link
Collaborator

Thanks @LukeTowers for this, I think that core inclusion will be a great opportunity and IMHO it could end in this awesome plugin's depreciation.

Do you plan to include also images' dimensions filters? https://github.com/toughdeveloper/oc-imageresizer-plugin/blob/develop/Plugin.php#L64-L75

In my use cases them are needful - ex: https://photoswipe.com/documentation/faq.html#image-size

@LukeTowers
Copy link
Contributor

@matteotrubini that would certainly be doable, I'll see if I can whip something up. It's a bit tricky because the new functionality supports remote disks, but I'll see what I can do.

@matt-pawley
Copy link
Owner

@LukeTowers - Great to see something happening with this plugin 👍 Thanks for taking the time to migrate the functionality to core.

Once it's merged and released, it would be my preference to try encourage users to remove this plugin in favour of the core functionality so they can receive new features, bug-fixes, etc.

The only drawback I can see of users making the switch is that it will invalidate the image cache and may lead to initial performance degradation.

How do you want to go about informing users? Possibly:

  • Update readme of this repository with deprecation notice and upgrade instructions
  • Push a major change with deprecation notice and upgrade instructions
  • Update readme within October CMS with deprecation notice and upgrade instructions
  • After a few months, remove the plugin from OctoberCMS.

@matteotrubini
Copy link
Collaborator

matteotrubini commented Aug 18, 2020

@LukeTowers save metadata's json file on resize? Using $path . '.metadata' you obtain strong relationship and this file can be managed by local cache as the orginal one.

@toughdeveloper I completely agree with you, And I would like to thank you once again for the work done during these years. 🤗

I change the title of the issue to make it more on-topic.
Once octobercms/october#5231 is merged we should close this issue to open a new one focused only on migration progress.

@LukeTowers
Copy link
Contributor

@toughdeveloper I would agree with that process, although I would remove the plugin from the marketplace when the functionality first releases, it's not going to remove it from any existing installs and new installs won't need to install it at all.

Upgrade instructions are pretty simple, basically you update the core and then run php artisan october:util purge uploads to purge the uploads directory of any files not referenced in the system_files table (as long as you aren't storing references to them elsewhere, and really this step is only to clean up your filesystem if you've been using it for a while).

@PictureElement
Copy link

I believe that the |resize filter already does the job of this plugin. The only missing feature is image optimization. It would be nice if it gets added to the core.

@LukeTowers
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants