-
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
enable ginkgolinter and fix findings #6421
enable ginkgolinter and fix findings #6421
Conversation
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
@nunnatsa looks like the scorecard tests are failing. At a glance looks like the changes in scorecard/ files is as expected. This may also be a flake, hence restarting the tests. |
@varshaprasad96 thsnks. I think this test is broken. It constantly fails in my other pr, and also locally from the master branch. |
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 this PR @nunnatsa! All the changes look good to me!
/lgtm
ad8e3aa
to
6e70778
Compare
@grokspawn @varshaprasad96 @everettraven - rebased to unblock the CI. Could you please approve again? |
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
Not sure closing and reopening is enough. I'll rebase |
@nunnatsa Closing and reopening worked. The current failures are unrelated and shouldn't block this PR |
Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>
6e70778
to
909a688
Compare
New changes are detected. LGTM label has been removed. |
rebased anyway. @everettraven - sorry - missed your coment. |
@varshaprasad96, @grokspawn - could you please take another look? |
/ok-to-test |
@grokspawn: /override requires failed status contexts, check run or a prowjob name to operate on.
Only the following failed contexts/checkruns were expected:
If you are trying to override a checkrun that has a space in it, you must put a double quote on the context. In response to this:
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. |
/override docs (see above) |
@grokspawn: Overrode contexts on behalf of grokspawn: docs, e2e-molecule In response to this:
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. |
🎉 |
Description of the change
enable ginkgolinter and fix findings.
Motivation for the change
Improve test code quality and prevent some bug, when using ginkgo and gomega.
Checklist
If the pull request includes user-facing changes, extra documentation is required:
changelog/fragments
(seechangelog/fragments/00-template.yaml
)website/content/en/docs