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

[PR] Remove Reason.ACQUIRE and Reason.RELEASE #288

Closed
4 of 5 tasks
kopf-archiver bot opened this issue Aug 18, 2020 · 0 comments
Closed
4 of 5 tasks

[PR] Remove Reason.ACQUIRE and Reason.RELEASE #288

kopf-archiver bot opened this issue Aug 18, 2020 · 0 comments
Labels
archive refactoring Code cleanup without new features added

Comments

@kopf-archiver
Copy link

kopf-archiver bot commented Aug 18, 2020

A pull request by Jc2k at 2020-01-06 12:49:51+00:00
Original URL: zalando-incubator/kopf#288
Merged by nolar at 2020-01-13 11:45:45+00:00

What do these changes do?

In #258 we talked about how there was a bit of a chicken and the egg situation with requires_finalizer. You proposed pulling the the requires_finalizer detection into handle_resource_changing_cause (option B).

This pull request implements option B from that discussion, as I understood it.

Description

The need to add or remove a handler was detected in detect_resource_changing_cause in causation.py.

The need to add or remove a handler will be detected in handle_resource_changing_cause in handling.py.

N/A

Internal.

nolar presented 2 possible options in another PR, this is the 2nd option.

Not that I can see, but i am not familiar with this code.

Issues/PRs

Issues:

Related:

#258

Type of changes

  • Refactoring (non-breaking change which does not alter the behaviour)

Checklist

  • The code addresses only the mentioned problem, and this problem only
  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • If you provide code modification, please add yourself to CONTRIBUTORS.txt

Commented by nolar at 2020-01-13 11:49:57+00:00
 

Jc2k Thanks for this PR. I guess, it now unblocks the way for the callback-filtering.

@kopf-archiver kopf-archiver bot closed this as completed Aug 18, 2020
@kopf-archiver kopf-archiver bot changed the title [archival placeholder] [PR] Remove Reason.ACQUIRE and Reason.RELEASE Aug 19, 2020
@kopf-archiver kopf-archiver bot added the refactoring Code cleanup without new features added label Aug 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
archive refactoring Code cleanup without new features added
Projects
None yet
Development

No branches or pull requests

0 participants