-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
refactor: move version rollback hook to event #50990
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
refactor: move version rollback hook to event #50990
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
c26b3bb to
e935ddd
Compare
susnux
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general it is quite nice now!
I would really like if you add a test to VersionManagerTest that checks the event is emitted correctly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some places in server using that event:
Util::connectHook('\OCP\Versions', 'rollback', $versionsActions, 'rollback'); \OC_Hook::connect('\OCP\Versions', 'rollback', $this->getWatcher(), 'versionRollback');
Can you migrate them ?
|
You need to apply the codestyle by running |
e935ddd to
9f35338
Compare
|
@artonge I can try. But it will be a separate PR since I need to allocate additional time to understand that code.
|
Would be nice, and a follow up is perfectly fine :) |
|
@come-nc I fixed the styles. Please run the tests again. |
https://github.com/nextcloud/server/actions/runs/13507438734/job/37768873708?pr=50990 |
Signed-off-by: ailkiv <[email protected]>
9f35338 to
9b3424f
Compare
@come-nc done |
|
@come-nc It seems that the failed checks are not related to the current PR. |
|
@sorbaugh cypress unrelated due to fork. Could you please merge? |
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
Summary
I'm working on a fix for the problem nextcloud/collectives#1602. In order not to do it through hooks, I decided to first rewrite the rollback hook to the event.
I also added backwards compatibility.
TODO
Checklist