Skip to content

Conversation

@Jamstah
Copy link
Contributor

@Jamstah Jamstah commented Jul 1, 2021

https://github.com/openshift/api/blob/a99ffa1cac6709edf8f502b16890b16f9a557e00/console/v1/types_console_yaml_sample.go

Signed-off-by: James Hewitt james.hewitt@uk.ibm.com

Description of the change:

Update string to match the real world

Motivation for the change:

Kind is not recognised because the case does not match the resource.

Reviewer Checklist

  • Implementation matches the proposed design, or proposal is updated to match implementation
  • Sufficient unit test coverage
  • Sufficient end-to-end test coverage
  • Docs updated or added to /docs
  • Commit messages sensible and descriptive

@openshift-ci openshift-ci bot requested review from ankitathomas and jmrodri July 1, 2021 13:31
@openshift-ci openshift-ci bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 1, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 1, 2021

Hi @Jamstah. Thanks for your PR.

I'm waiting for a operator-framework member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@codecov
Copy link

codecov bot commented Jul 1, 2021

Codecov Report

Merging #694 (9f054fc) into master (86907e1) will decrease coverage by 1.79%.
The diff coverage is n/a.

❗ Current head 9f054fc differs from pull request most recent head 423d966. Consider uploading reports for the commit 423d966 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #694      +/-   ##
==========================================
- Coverage   50.37%   48.57%   -1.80%     
==========================================
  Files         102       95       -7     
  Lines        8702     8096     -606     
==========================================
- Hits         4384     3933     -451     
+ Misses       3469     3407      -62     
+ Partials      849      756      -93     
Impacted Files Coverage Δ
pkg/lib/bundle/supported_resources.go 100.00% <ø> (ø)
pkg/configmap/configmap_writer.go 0.00% <0.00%> (-33.11%) ⬇️
pkg/lib/registry/registry.go 13.33% <0.00%> (-10.27%) ⬇️
pkg/lib/indexer/indexer.go 6.90% <0.00%> (-3.88%) ⬇️
pkg/sqlite/load.go 34.47% <0.00%> (-3.65%) ⬇️
pkg/server/server.go 43.58% <0.00%> (-3.47%) ⬇️
pkg/lib/bundle/validate.go 60.35% <0.00%> (-0.35%) ⬇️
pkg/registry/empty.go 0.00% <0.00%> (ø)
pkg/sqlite/deprecate.go 0.00% <0.00%> (ø)
... and 37 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 86907e1...423d966. Read the comment docs.

@Jamstah
Copy link
Contributor Author

Jamstah commented Jul 1, 2021

I switched this to change just the value and not the constant, although I expect the current constant isn't being used (as its wrong anyway), so it might be better to make the breaking API change instead.

@Jamstah Jamstah force-pushed the cys-case-fix branch 2 times, most recently from b3d6765 to 2f9bd14 Compare July 1, 2021 13:51
@Jamstah
Copy link
Contributor Author

Jamstah commented Jul 1, 2021

Hah, changing the value is considered incompatible too, so I'm going to go ahead and say "we should make an incompatible change here".

@Jamstah Jamstah changed the title Update CASE to match resource. Update case to match resource. Jul 1, 2021
@dinhxuanvu
Copy link
Member

/ok-to-test

@openshift-ci openshift-ci bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 2, 2021
@dinhxuanvu
Copy link
Member

Can you please rebase this PR against latest master? We recently fixed a kind test on CI. Thanks.

@Jamstah Jamstah force-pushed the cys-case-fix branch 2 times, most recently from 2fb7e9d to 7304fda Compare July 4, 2021 22:30
@Jamstah
Copy link
Contributor Author

Jamstah commented Jul 4, 2021

Can you please rebase this PR against latest master? We recently fixed a kind test on CI. Thanks.

Done.

@Jamstah
Copy link
Contributor Author

Jamstah commented Jul 5, 2021

/assign @dinhxuanvu

Assigning because you already seem to be looking at it

@timflannagan
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 22, 2021
@timflannagan
Copy link
Member

Related: #757.

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 1, 2021
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Sep 1, 2021
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 1, 2021
@Jamstah
Copy link
Contributor Author

Jamstah commented Sep 1, 2021

@timflannagan rebased, can I have the lgtm back?

Copy link
Member

@dinhxuanvu dinhxuanvu left a comment

Choose a reason for hiding this comment

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

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 1, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dinhxuanvu, Jamstah

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 1, 2021
@Jamstah
Copy link
Contributor Author

Jamstah commented Sep 23, 2021

@timflannagan I had to rebase this so it lost your LGTM, can you please provide another?

@timflannagan
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 23, 2021
@openshift-merge-robot openshift-merge-robot merged commit 0ea069e into operator-framework:master Sep 23, 2021
akihikokuroda pushed a commit to akihikokuroda/operator-registry that referenced this pull request Sep 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants