-
Notifications
You must be signed in to change notification settings - Fork 472
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
Beta resources: Setting storage version to v1beta1, not serving v1alpha2 #1348
Conversation
This is a big change. Adding a hold for consensus. /hold |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, this was a surprisingly big one. Most of the changes are pretty straightforward renames, but this probably needs someone to do another visual check through the docs and make sure they make sense (I am going a bit crosseyed).
/lgtm
@@ -106,6 +105,9 @@ for CHANNEL in experimental standard; do | |||
echo Examples failed as expected | |||
done | |||
|
|||
# Undo workaround from earlier |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooh, this is a good catch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Temporary workaround for release" sounds like something that should just be removed? If it's needed permanently, maybe we need to update the comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're right, this probably shouldn't exist at all. There was a time where we wanted the manifests installed from the main branch to be at least somewhat useful and refer to the latest released image instead of the latest image build from the main branch. All of our installation instructions clearly refer to a versioned release so this is likely entirely unnecessary, will revert this change altogether.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually as I was digging into this more, it looks more complicated than initially expected and I don't want to expand this PR any further. I've created #1366 to track this and added it to the v0.6.0 milestone.
This needs updating in
|
/lgtm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: robscott, shaneutt The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Fixes #1372 /hold cancel |
This affects resources that graduated to v1beta1 in the v0.5.0 release. Although that specific change is relatively small, it required a lot of related work: * Restructuring examples to remove v1alpha2 examples where they were no longer useful and their installation would fail * Updating conformance tests to use v1beta1 where possible * Some related fixes to v1alpha2 blog post to account for removal of v1alpha2 example
521bb71
to
5f296d8
Compare
Missed updating one conformance test to use v1beta1, PTAL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
What type of PR is this?
/kind cleanup
/kind deprecation
What this PR does / why we need it:
This affects resources that graduated to v1beta1 in the v0.5.0 release.
Although that specific change is relatively small, it required a lot of
related work:
longer useful and their installation would fail
v1alpha2 example
This is closely related to #1347
Does this PR introduce a user-facing change?: