Skip to content
This repository has been archived by the owner on Jan 8, 2024. It is now read-only.

Add support for project-level events #4861

Merged
merged 4 commits into from
Aug 7, 2023

Conversation

paladin-devops
Copy link
Contributor

No description provided.

This is necessary to enable "project-level" events. Before this commit, Events could only be used at the "app-level".
@paladin-devops paladin-devops requested a review from a team July 27, 2023 17:28
@paladin-devops paladin-devops self-assigned this Jul 27, 2023
@paladin-devops paladin-devops marked this pull request as ready for review July 27, 2023 17:28
@paladin-devops paladin-devops requested a review from a team as a code owner July 27, 2023 17:28
@@ -845,7 +849,12 @@ message UI {
uint64 deployment_sequence = 6;
}

message EventAddOn {
message EventAddOnCreated {
Copy link
Contributor

@andrew-hashicorp andrew-hashicorp Jul 27, 2023

Choose a reason for hiding this comment

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

I'm curious what the rationale is for splitting out EventAddonCreated and EventAddonDestroyed into 2 separate events. My first instinct is what you had initially (EventAddOn), with the created/destroyed type being inside EventData.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andrew-hashicorp I think that your suggestion is a good stylistic suggestion on how "events" should be designed. Another example that could fit this model (in the future) is the "deployment" event, which could either be the creation or destruction of a deployment.

I am merging this PR to the non-main upstream branch in its current state, but will follow-up with a PR containing this suggested change as well.

@paladin-devops paladin-devops merged commit 02088ab into f-add-ons-serverstate Aug 7, 2023
35 of 36 checks passed
@paladin-devops paladin-devops deleted the i-project-events branch August 7, 2023 14:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants