-
Notifications
You must be signed in to change notification settings - Fork 6
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
Release notes url in wide output #510
Conversation
crReleaseNotesURLAnnotation = "giantswarm.io/release-notes" | ||
kindRelease = "Release" | ||
releaseDocumentationLink = "https://docs.giantswarm.io/reference/cp-k8s-api/releases.release.giantswarm.io/" | ||
releaseNotesLink = "https://github.com/giantswarm/releases/tree/master/kvm/v11.2.0" |
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.
Maybe the default should be an empty string or https://github.com/giantswarm/releases
.
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 would prefer to just link to the repo as well :)
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.
Ok just so that we're on the same page: Once I change the value here, I also need to change it in config/crd/v1/release.giantswarm.io_releases.yaml
to satisfy the tests.
Is the value here a default value for the release CR?
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.
Yes, exactly. We create release CRs with helm+kustomize instead of operators so this is unlikely to come up in the near future anyways (i.e. this code won't be executed anywhere except in the test).
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.
Thanks for the explanation. I'll change this now.
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.
Small nit - looking nice otherwise 🚀
crReleaseNotesURLAnnotation = "giantswarm.io/release-notes" | ||
kindRelease = "Release" | ||
releaseDocumentationLink = "https://docs.giantswarm.io/reference/cp-k8s-api/releases.release.giantswarm.io/" | ||
releaseNotesLink = "https://github.com/giantswarm/releases/tree/master/kvm/v11.2.0" |
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 would prefer to just link to the repo as well :)
This PR adds a new column to
kubectl get releases -o wide
output.I am unsure why the
AGE
column is different to output on live installations. I've tested this with a local kind cluster (k8s 1.18.2)Checklist