Skip to content
This repository has been archived by the owner on Sep 14, 2020. It is now read-only.

Remove Reason.ACQUIRE and Reason.RELEASE #288

Merged
merged 1 commit into from
Jan 13, 2020
Merged

Remove Reason.ACQUIRE and Reason.RELEASE #288

merged 1 commit into from
Jan 13, 2020

Conversation

Jc2k
Copy link
Contributor

@Jc2k Jc2k commented Jan 6, 2020

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

@Jc2k Jc2k requested a review from nolar as a code owner January 6, 2020 12:49
@zincr
Copy link

zincr bot commented Jan 6, 2020

🤖 zincr found 1 problem , 0 warnings

❌ Specification
✅ Large Commits
✅ Approvals
✅ Dependency Licensing

Details on how to resolve are provided below


Specification

All pull requests must follow certain rules for content length and form

Please ensure the follow issues are resolved:

  • ✅ Pull Request Title must be atleast 8 characters
  • ✅ Pull Request body must be atleast 8 characters
  • ❌ Pull Request body must contain issue number
     

@Jc2k Jc2k mentioned this pull request Jan 6, 2020
2 tasks
@nolar nolar added the refactoring Code cleanup without new features added label Jan 13, 2020
@nolar nolar requested review from aweller and samurang87 January 13, 2020 10:46
@nolar nolar merged commit 5e29dc0 into zalando-incubator:master Jan 13, 2020
@nolar
Copy link
Contributor

nolar commented Jan 13, 2020

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
refactoring Code cleanup without new features added
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants