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

update Hooks to Symfony 5 #4593

Merged
merged 45 commits into from
Feb 24, 2021
Merged

update Hooks to Symfony 5 #4593

merged 45 commits into from
Feb 24, 2021

Conversation

craigh
Copy link
Member

@craigh craigh commented Feb 7, 2021

This includes temporary example files in /App and modifies the following files in order to achieve those:

These files should be changed back when testing is complete.

  • composer.json
  • config/services.yaml
  • config/routes/annotations.yaml

These files should be removed:

  • src/Controller/TestHookController.php (the template file for this controller is not included)
  • src/HookEvent/*
  • src/HookListener/*
  • src/Form/*
Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? YES
Fixed tickets @see linked issues
Refs tickets @see linked issues
License MIT
Changelog updated yes

Todos

  • Tests - for HookLocator, Connection?
  • Documentation
  • Changelog
  • Code
    • add ConnectionPostChangeEvent
    • fix installer to include new code (check HookBundleInstaller and CoreInstallerListener)
    • update HooksListener to add link to new UI
    • adapt ModuleInstallListener to new code (what happens if code not in an extension?)
    • move example code from App to docs

@craigh

This comment has been minimized.

@Guite

This comment has been minimized.

@craigh

This comment has been minimized.

@craigh
Copy link
Member Author

craigh commented Feb 10, 2021

Screen Shot 2021-02-10 at 4 00 12 PM

@craigh

This comment has been minimized.

@Guite

This comment has been minimized.

This includes temporary example files in `/App` and modifies the following files in order to achieve those:

composer.json
config/services.yaml
config/routes/annotations.yaml

These files should be changed back when testing is complete.
…nterface on each event, autoconfigure each event.

Add beginning of new connections controller
@Guite

This comment has been minimized.

@craigh

This comment has been minimized.

@Guite

This comment has been minimized.

@craigh craigh marked this pull request as ready for review February 24, 2021 01:15
@craigh craigh requested a review from Guite February 24, 2021 01:15
Copy link
Member

@Guite Guite left a comment

Choose a reason for hiding this comment

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

Two questions:

  1. How is the connection entity's table going to be created if the hook bundle will be used in a non-Zikula environment? Flex recipe? Auto detection? (of course this is outside of this PR).

  2. Do you have considered a place in UI where the user can see the summary/information about a hook (like described in [HookBundle] Helppage for each Hook #4398)? Didn't find it.

Otherwise go for it 👍 👍 🍰 🥇

@craigh
Copy link
Member Author

craigh commented Feb 24, 2021

How is the connection entity's table going to be created if the hook bundle will be used in a non-Zikula environment? Flex recipe? Auto detection? (of course this is outside of this PR).

There is a new schema:create command in this PR that could be used, but ultimately, that would be the problem of the developer implementing it..

Do you have considered a place in UI where the user can see the summary/information about a hook (like described in #4398)? Didn't find it.

There is a list under the UI with all the HookEvents and HookEventListeners.

@craigh craigh merged commit 5c2bc58 into main Feb 24, 2021
@craigh craigh deleted the _hooks branch February 24, 2021 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants