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.

Sign up for GitHub

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

Jump to bottom

Bring the structs in line with the CRDs. #1198

Merged
merged 3 commits into from
Dec 19, 2019
Merged

Bring the structs in line with the CRDs. #1198

merged 3 commits into from
Dec 19, 2019

Conversation

porridge
Copy link
Member

What this PR does / why we need it:

This makes the description match (missed in on of the earlier PRs) and adds a
missing field.

This is a baby step towards #862.

alenkacz
alenkacz requested changes Dec 18, 2019
View reviewed changes
Copy link
Contributor

@alenkacz alenkacz left a comment

Choose a reason for hiding this comment

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

See my comment :)

pkg/apis/kudo/v1beta1/operatorversion_types.go Outdated
@@ -46,6 +46,8 @@ type OperatorVersionSpec struct {

// UpgradableFrom lists all OperatorVersions that can upgrade to this OperatorVersion.
UpgradableFrom []OperatorVersion `json:"upgradableFrom,omitempty"`

CrdVersion string `json:"crdVersion,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

huh? What is CrdVersion? Is that probably something we should rather remove from CRD than add in here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you think we could remove it from the CRD without having to bump to v1beta2?

Copy link
Contributor

@alenkacz alenkacz Dec 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I personally think it's a safe thing to do if we make sure that the field was not used in 0.8.0 and 0.9.0 - before that, we did not provide backward compatibility anyway so it does not matter (but I doubt it was used ever...). I don't even know what would be the purpose of this to be honest.

Copy link
Member Author

@porridge porridge Dec 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I looked at the history (git log -p) all the way to the very beginning and it seems it was never used.
Reverted that bit here, and will send another PR to remove it from the CRDs.
PTAL @alenkacz

Copy link
Member Author

Choose a reason for hiding this comment

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

Created PR #1204

@porridge porridge mentioned this pull request Dec 18, 2019
Merged
alenkacz
alenkacz approved these changes Dec 18, 2019
View reviewed changes
Copy link
Contributor

@alenkacz alenkacz left a comment

Choose a reason for hiding this comment

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

@porridge please update the title of this before you merge otherwise LGTM

@porridge porridge merged commit ef9d22d into master Dec 19, 2019
@porridge porridge deleted the crd-correction-structs branch December 19, 2019 06:53
porridge added a commit that referenced this pull request Dec 19, 2019
@porridge
Remove the unused crdVersion from CRD. (#1204)
ff75b96 
Result of discussion in #1198.

It was never used (verified by eyeballing git log -p all the way
down), we don't seem to know what it's for, and it has no counterpart in
the structs.

This is a baby step towards #862.
ANeumann82 pushed a commit that referenced this pull request Feb 13, 2020
@porridge @ANeumann82
Update field description to match the one in kudoinit. (#1198)
a8b84b9 
This was missed in #1172

Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>
ANeumann82 pushed a commit that referenced this pull request Feb 13, 2020
@porridge @ANeumann82
Remove the unused crdVersion from CRD. (#1204)
2d7c279 
Result of discussion in #1198.

It was never used (verified by eyeballing git log -p all the way
down), we don't seem to know what it's for, and it has no counterpart in
the structs.

This is a baby step towards #862.

Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alenkacz alenkacz alenkacz approved these changes

@gerred gerred Awaiting requested review from gerred gerred is a code owner

@kensipe kensipe Awaiting requested review from kensipe kensipe is a code owner

@nfnt nfnt Awaiting requested review from nfnt nfnt is a code owner

@zen-dog zen-dog Awaiting requested review from zen-dog zen-dog is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@porridge @alenkacz