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

Migrate Subscribers (event handlers) #1918

Merged
merged 5 commits into from
Mar 19, 2025

Conversation

Manas-Kenge
Copy link
Contributor

@Manas-Kenge Manas-Kenge commented Mar 19, 2025

@Manas-Kenge
Copy link
Contributor Author

@jensens I’ve opened a separate PR with your requested changes. Feel free to review it when you have time!

Copy link
Contributor

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

Someone with knowledge of the subscriber system needs to make this cohesive. I don't know which content is authoritative or derivative.

@stevepiercy
Copy link
Contributor

@plone/classicui-team halfway through my review, I gave up. Would someone please provide an assist? See #1918 (review)

@Manas-Kenge
Copy link
Contributor Author

@petschki I understand what you are saying. The content is taken from Event handlers and Events which are similar. I thought it would be a good idea to include both of them. Should I remove Event handlers?

@petschki
Copy link
Member

@petschki I understand what you are saying. The content is taken from Event handlers and Events which are similar. I thought it would be a good idea to include both of them. Should I remove Event handlers?

From my point of view yes. This is redundant information.

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

@stevepiercy stevepiercy merged commit 379ef2e into plone:6.0 Mar 19, 2025
2 checks passed
@stevepiercy
Copy link
Contributor

Thank you!

@stevepiercy stevepiercy mentioned this pull request Mar 19, 2025
40 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants