-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
handle copy of live photos #42170
handle copy of live photos #42170
Conversation
bb1aaad
to
f485344
Compare
f485344
to
4ab8075
Compare
/backport to stable28 |
4ab8075
to
aa4b3f0
Compare
/** | ||
* @since 20.0.0 | ||
*/ | ||
public function __construct(Node $source, Node $target, private bool &$run) { |
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.
storing a reference seems pretty hacky to me and an easy way to introduce weirdness down the line.
I would prefer to pass $run
by value and then update $arguments['run']
after the hook is dispatched.
It's more code but the intent is clearer to me.
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.
How can the listener gives the new value of $run
back to the emitter?
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.
add a getter so HookConnector
can retrieve it
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.
That sounds smart. @ArtificialOwl will you have the time to update the other event listeners too ? :D
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.
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.
Tested few other ways but the simplest seems to have the HookController catching new event AbortedEventException
and update $arguments['run']
. Throwing the exception also stop the event propagation.
@icewind1991 @artonge please re-review
aa4b3f0
to
f81adce
Compare
thanks @icewind1991 |
f81adce
to
dac60d9
Compare
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
dac60d9
to
467c84e
Compare
The backport to # Switch to the target branch and update it
git checkout stable28
git pull origin stable28
# Create the new backport branch
git checkout -b backport/42170/stable28
# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 467c84ec
# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/42170/stable28 Error: No changes found in backport branch Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports. |
@artonge based on the size of the changes and the need for this feature, do we want to backport to 28 ? |
I would say no, but not for me to decide, @sorbaugh |
Hi. Just updated to 29.0.3 and the error persists : |
BeforeNodeCopiedEvent
to check the doability of copying both files.NodeCopiedEvent
to generate a copy of the linked file.