Skip to content

Make ApplicationSpec.Destination non-optional, non-pointer#177

Merged
merenbach merged 4 commits intoargoproj:masterfrom
merenbach:require-destination
May 8, 2018
Merged

Make ApplicationSpec.Destination non-optional, non-pointer#177
merenbach merged 4 commits intoargoproj:masterfrom
merenbach:require-destination

Conversation

@merenbach
Copy link
Contributor

Closes #175

@jessesuen is this what we had in mind? Any other changes/additions? I note a mention of immutability—should that be filtering with a conditional upon update attempts?

@merenbach merenbach added the help-wanted Extra attention is needed label May 8, 2018
@merenbach merenbach requested a review from jessesuen May 8, 2018 18:37
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this function anymore? I think only Create app needs this logic.

Copy link
Member

Choose a reason for hiding this comment

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

I searched usages of this and I think we should try removing this since we can now safely assume that server/namespace will be filled out in the object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed an update that removes this function.

@jessesuen
Copy link
Member

@jessesuen is this what we had in mind?
yes this is precisely what I was thinking.

@jessesuen
Copy link
Member

Thanks!

@merenbach
Copy link
Contributor Author

@jessesuen

It seems that perfection is attained not when there is nothing more to add, but when there is nothing more to remove. – Antoine de Saint Exupéry

@merenbach merenbach merged commit b83eac5 into argoproj:master May 8, 2018
@merenbach merenbach deleted the require-destination branch May 8, 2018 21:09
leoluz pushed a commit to leoluz/argo-cd that referenced this pull request Sep 29, 2023
…nc wave (argoproj#177)

Signed-off-by: Jesse Suen <jesse_suen@intuit.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

help-wanted Extra attention is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants