-
Notifications
You must be signed in to change notification settings - Fork 191
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
Fix missing properties on UpdateOperation #748
Conversation
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.
Thanks! This will need tests to get merged, please? Also amend your commits with -s
for DCO to pass.
Signed-off-by: Christian Winkler <[email protected]>
Signed-off-by: Christian Winkler <[email protected]>
Signed-off-by: Christian Winkler <[email protected]>
Signed-off-by: Christian Winkler <[email protected] Signed-off-by: Christian Winkler <[email protected]>
Signed-off-by: Christian Winkler <[email protected] Signed-off-by: Christian Winkler <[email protected]>
The DCO thing is really a pain. |
Yeah you get used to it :) It's really a small price to pay to make non-Engineers happy. |
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.
Thanks for hanging out with us @fs-chris!
Oh crap I hit merge but just realized this PR was made into the 2.8.1 branch :( That wasn't supposed to happen. I've reverted via a force push and opened #750 as a copy of this. |
@fs-chris You need to make this PR into |
I cannot use So, please, anyone else to do this in Thanks... |
@fs-chris I see. It's just about order of operations. Make the PR in main, then (we will) backport it into 2.x branch instead of 2.8.1. Without making a PR in main the fix will be gone from the next major release. |
Hope someone wants to pick this up. We definitely appreciate your work, and as much time as you're willing to spend on this bug! |
Description
Adds missing properties
scriptedUpsert
anddetectNoop
toUpdateOperation
Issues Resolved
Fixes #744
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.