Closed gforcada closed 2 years ago
Thanks for this Gil!
It's early morning here in the US, so before I dive in to the review, can I (get coffee) and ask why a utility is better suited than a tool?
(answer: because @cewing says it's a good idea #15 )
I want to really understand your thoughts, the problem, and your solution. If anything, I want to learn from this myself!
Do you have a goal here or is this cleanup for future-proofing?
I'm down for the change, this is very old stuff, just walk me through your thoughts.
I'm excited this is getting more love.
@flipmcf good morning! 🌞 get your ☕ don't worry 😄
As for why a utility is better, sure @cewing said so ™️ but in general IMHO I would say that not having persistent content is always more future proof than keeping objects around? Specially if they come from an add-on that one might install, use it and at some point forgot about it, get it removed and eventually, on a catalog cleanup and reindex one gets bitten by a ModuleNotFound
? 😅
My goal mostly is to not get a persistent object that might bite back in the future. We have a website with +80k keywords and we are looking at ways to shrink that number to a size that feels comfortable.
For us, keywords are important for SEO reasons, for internal searches, but also for a "related items" code that we are building around keywords, so the better they are kept, the better the suggestions they will be.
I'm building a script to extract and process the keywords so I can give an easy to use interface to our editors to do a first major cleanup, and then, I plan to install this on production so they can do maintenance work on their own.
I might try to contribute this script in an extension of the control panel, not sure if it would be useful or not 🤔
Since PloneKeywordManager does not store anything, a non persistent utility is perfect. At the time of writing (about 19 years ago) this concept was not born at all. That is the reason why it is as it is. I think this is nice step of modernization and I am +1 to merge this.
I even would skip any deprecation warning and just go for a new major version.
Anyway, an upgrade step is absolutely mandatory, otherwise we are running Plone sites with a zombie object.
@jensens deprecations not added, and upgrade step added 👍🏾
@jensens thanks for merging! may we make a new release? 😄 I can cut it, though I don't have access on pypi for it. My username on pypi is gilforcada
.
@gforcada invite sent!
Thanks! I created 5.0.0a1 @flipmcf as you have it already on production, I guess, could you try the upgrade step and check that everything works fine? I will do the same with a fresh new installation as well 😄
Fixes #15
Turn the persistent tool into a utility 🎉
As a bonus I did quite a few cleanups, l made each of them in a separate commit to easily review and revert any if it is not meant to get merged
Todos: