-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Use scorecard v1alpha3 #3208
Use scorecard v1alpha3 #3208
Conversation
9527b50
to
b87bcda
Compare
@@ -697,6 +697,7 @@ github.com/operator-framework/operator-sdk v0.17.0 h1:+TTrGjXa+lm7g7Cm0UtFcgOjnw | |||
github.com/operator-framework/operator-sdk v0.17.0/go.mod h1:wmYi08aoUmtgfoUamURmssI4dkdFGNtSI1Egj+ZfBnk= |
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.
This file should be gitignore'd
internal/scorecard/alpha/run_test.go
Outdated
mockResult.Name = "mocked test" | ||
mockResult.Description = "mocked test description" | ||
mockResult.State = v1alpha2.PassState | ||
// mockResult.Description = "mocked test description" |
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.
Can this be removed?
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.
Knew I'd miss one
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.
b87bcda
to
2fa3abf
Compare
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.
the project is not building. See: https://travis-ci.org/github/operator-framework/operator-sdk/jobs/697332962#L941
@@ -103,6 +103,7 @@ images/scorecard-test/scorecard-test | |||
pkg/ansible/runner/testdata/valid.yaml | |||
/bin | |||
test/test-framework/go.sum | |||
internal/scorecard/alpha/examples/custom-scorecard-tests/go.sum | |||
|
|||
# Website | |||
website/public/ |
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.
should we not add a fragment for that?
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 don't think so, shouldn't have any impact on the user. Fragment probably required for some other stuff htough
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.
is not it changing the json output? see here
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.
It looks like that link just takes me back to this conversation
s/ScorecardTestResult/TestResult s/ScorecardOutput/Test s/output.Results/output.Status.Results Fix errors in formatting.go s/testOutput.Results/testOutput.Status.Results Unit tests now passing Remove unused description argument Description has been removed Update scorecard alpha docs
0cf4d59
to
94aaacf
Compare
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.
Great work 👍 . just maybe the nit of the fragment otherwise
/lgtm
/approve
@@ -25,30 +25,29 @@ const ( | |||
) | |||
|
|||
// CheckSpecTest verifies that CRs have a spec block | |||
func CheckSpecTest(bundle *apimanifests.Bundle) scapiv1alpha2.ScorecardTestResult { | |||
r := scapiv1alpha2.ScorecardTestResult{} | |||
func CheckSpecTest(bundle *apimanifests.Bundle) scapiv1alpha3.TestResult { |
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.
we want with v1alpha3 for a test to be able to return multiple test results instead of just a single result as before. So instead of returning TestResult, all the tests (basic/olm) should return a TestStatus, in cases like basic/olm where only a single test result is created, you end up with a TestStatus that has Results length = 1.
New changes are detected. LGTM label has been removed. |
Description of the change:
We will now use the newly defined Scorecard v1alpha3 API instead of v1alpha2
Motivation for the change:
v1alpha2 has been superseded by v1alpha3