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

Add process readiness changed controller and event #3554

Merged
merged 1 commit into from
Jan 11, 2024
Merged

Conversation

ameowlia
Copy link
Member

@ameowlia ameowlia commented Dec 14, 2023

See more context here: cloudfoundry/capi-release#363

  • I have reviewed the contributing guide

  • I have viewed, signed, and submitted the Contributor License Agreement

  • I have made this pull request to the main branch

  • I have run all the unit tests using bundle exec rake

  • I have run CF Acceptance Tests

@sethboyles
Copy link
Member

is this waiting on any other PRs before we can merge?

@sethboyles sethboyles self-requested a review January 4, 2024 18:36
@ameowlia
Copy link
Member Author

ameowlia commented Jan 8, 2024

@sethboyles - All of the PRs are listed here. I just noticed one of them for the runtime schema is failing some checks. I will look into it.

@@ -15,6 +15,21 @@ class AppEventRepository
docker_credentials].freeze
SYSTEM_ACTOR_HASH = { guid: 'system', type: 'system', name: 'system', user_name: 'system' }.freeze

def create_app_readiness_changed_event(app, readiness_changed_payload)
Copy link
Contributor

@dalvarado dalvarado Jan 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey @ameowlia, where is this function being called, @sethboyles and i couldn't find it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My guess is that I forgot to delete that code. I initially (erroneously) added an app event instead of an app.process event. It's probably part of the app event that I should've deleted. I'll test to make sure everything still works if I remove it tomorrow. Good catch!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the code. Units still pass. And when I deployed it everything still works.

@ameowlia ameowlia force-pushed the readiness-hc-events branch 3 times, most recently from 9f965d3 to c216d2c Compare January 10, 2024 17:00
Co-authored-by: Geoff Franks <[email protected]>
Signed-off-by: Geoff Franks <[email protected]>
@ameowlia ameowlia force-pushed the readiness-hc-events branch from c216d2c to a563539 Compare January 11, 2024 17:00
Copy link
Member

@moleske moleske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

woot!

@moleske moleske merged commit 06ca46a into main Jan 11, 2024
9 checks passed
@moleske moleske deleted the readiness-hc-events branch January 11, 2024 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants