Skip to content
This repository has been archived by the owner on Apr 26, 2021. It is now read-only.

Silences: Use PostableSilence as the base struct for creating silences #38

Merged
merged 4 commits into from
Mar 30, 2021

Conversation

gotjosh
Copy link
Contributor

@gotjosh gotjosh commented Mar 30, 2021

No description provided.

@@ -93,7 +93,7 @@ import (
// swagger:parameters RouteCreateSilence
type CreateSilenceParams struct {
// in:body
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Upstream uses the same nomenclature but unsure whenever we've deliberately changed it or this was unintended.

/ swagger:parameters postSilences
type PostSilencesParams struct {

	// HTTP Request Object
	HTTPRequest *http.Request `json:"-"`

	/*The silence to create
	  Required: true
	  In: body
	*/
	Silence *models.PostableSilence
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think it was deliberately changed. @domasx2 can you confirm?

type PostableSilence = amv2.PostableSilence

// swagger:model
type GettableSilences = amv2.GettableSilences
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From a brief research, I was unsure whenever there would be any downsides of using type aliasing vs struct embedding. In this case, I don't want to add any extra methods so I went with the former. Happy to listed to opinions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't have preference, if aliasing works better for encoding/decoding purposes let's keep that.

Copy link
Collaborator

@papagian papagian left a comment

Choose a reason for hiding this comment

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

LGTM, you have to push also the modifications in spec.json and post.json

type PostableSilence = amv2.PostableSilence

// swagger:model
type GettableSilences = amv2.GettableSilences
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't have preference, if aliasing works better for encoding/decoding purposes let's keep that.

@gotjosh gotjosh merged commit 0b5408c into master Mar 30, 2021
@gotjosh gotjosh deleted the postable-silence branch March 30, 2021 16:22
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.

None yet

2 participants