-
Notifications
You must be signed in to change notification settings - Fork 84
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
Deserialization cleanup, test simplification, coverage expansion #823
base: master
Are you sure you want to change the base?
Deserialization cleanup, test simplification, coverage expansion #823
Conversation
Skipping CI for Draft Pull Request. |
The new mockoon tests did catch an issue here, but that made me see two things:
I'll address both of those separately. |
8c5c375
to
f1ce6c1
Compare
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: Kevin M Granger <[email protected]>
f1ce6c1
to
f446fe0
Compare
Test images available! 🧪🚀 To test operator with them, run
To clean up environment afterwards, run
|
This is good enough as-is, and more tests can be added later. |
@mateusoliveira43 I'm not familiar with the workflow for updating the chart patch versions. But isn't that only supposed to happen on a release anyway? |
@KevinMGranger we changed to apply the change to each PR, ref https://pelorus.readthedocs.io/en/latest/Development/#versioning-process (I think there was a discussion on this, but do not have the link) on release, we bump |
I'm trying to do that, but it can't find the I try to re-run
But it still exits with a 0 status 🤔 |
yeah, that is a known issue #518 try running script directly with for example |
$ ./scripts/update_projects_version.py -f -r
INFO:root:Removing destination folder /Users/kevin/workcode/pelorus/pelorus-operator
ERROR:root:You must be logged in to a OpenShift cluster as a user with cluster-admin permissions to run this script. This avoids RBAC problems. More info: https://sdk.operatorframework.io/docs/building-operators/helm/tutorial/#prerequisites I don't see why this is necessary, perhaps we should open an issue for this. I'll get my CRC cluster up and running again I suppose. |
I think there is missing explanation in Operator SDK site, but it is in there prerequisites maybe with the PR I am working on #1045 we can remove this constrain, but today I believe we need it |
Signed-off-by: Kevin M Granger <[email protected]>
Test images available! 🧪🚀 To test operator with them, run
To clean up environment afterwards, run
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@KevinMGranger: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
related to #757