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

Change the registry.xml to use plone.base.interfaces.resources.IBundleRegistry #194

Merged
merged 8 commits into from
Aug 28, 2024

Conversation

cihanandac
Copy link
Contributor

Related to Issue 193

Changed the registry.xml to use "plone.base.interfaces.resources.IBundleRegistry" instead of "Products.CMFPlone.interfaces.IBundleRegistry"

@cihanandac cihanandac changed the title fix(IBundleRegistery): change the registery to use plone.base.interfa… Change the registry.xml to use plone.base.interfaces.resources.IBundleRegistry Aug 28, 2024
Copy link
Member

@petschki petschki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM in general but please add a note in CHANGES.rst as we do not use towncrier here.

@cihanandac
Copy link
Contributor Author

I added a news item as it is suggested in the CHANGES.rst, I hope this is what you suggested also @petschki.

@petschki
Copy link
Member

oh, sorry your're right ... we do use towncrier actually. perfect 👍🏼

I'll use your PR to do some housekeeping too if you don't mind (JS dependency updates) then I'll merge

Copy link
Member

@petschki petschki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, you've forked the repo. So lets merge this and do the housekeeping separately.

It would be better to create a PR directly on collective so that other users can change things too ... otherwise you would have to grant access to your repo... but that's for the next one 😉

@cihanandac
Copy link
Contributor Author

I see your point, I will keep this in mind for the next time:)
Thank you for everything.

Copy link
Member

@petschki petschki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, two more things:

  1. would you please change the interface location also in the uninstall profile ... just the same like in default ...
  2. we need an upgrade step to version 6 with the new registry interface ... see the upgrades folder. basically the same like its done in 5 without the removals.

@cihanandac
Copy link
Contributor Author

Changed the uninstall profile as well and also included version 6 to the upgrades as requested.
@petschki I have added you as the collaborator to the fork incase if you want to make the housekeeping on it that you mentioned.

@petschki petschki merged commit dfdf237 into collective:master Aug 28, 2024
6 checks passed
@petschki
Copy link
Member

Thanks! you can remove my access from your repo now ...

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

Successfully merging this pull request may close these issues.

2 participants