✨ add testify linter and address fixes #1334
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1334 +/- ##
=======================================
Coverage 76.37% 76.37%
=======================================
Files 41 41
Lines 2438 2438
=======================================
Hits 1862 1862
Misses 402 402
Partials 174 174
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
@grokspawn did you run testifylint locally and the PR code changes are driven by the output of the run? |
|
Also the commit does not have any details about why this PR was required i.e. some historical context around #1330 would also help. |
|
/hold Adding hold to get some response before we merge this PR. |
openshift-ci
bot
added
the
do-not-merge/hold
|
/hold cancel as we discussed this in slack. |
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
grokspawn
force-pushed
the
testify_linter
branch
from
06009e7
to
1d7c603
Compare
Of course. |
What are you looking for? The description of this PR already gives a little background on how this evolved from the other PR and includes a link to it. |
This PR description has useful information/context but the commit does not have it. we should not have to rely on the the github interface to find useful information in general. |
While reviewing operator-framework#1330 we had a discussion about the ability to generally prevent the use of `requires` within `Eventually` calls to prevent future flakes, and that led us to evaluate the `testifylint`-er to give us some cautionary guidelines. It did _not_ resolve the requires-within-eventually issue, but we may try to tackle that with some custom AST in the future. Signed-off-by: Jordan Keister <jordan@nimblewidget.com>
|
New changes are detected. LGTM label has been removed. |
Description
While reviewing #1330 we had a discussion about the ability to generally prevent the use of
requireswithinEventuallycalls to prevent future flakes, and that led us to evaluate thetestifylint-er to give us some cautionary guidelines. It did not resolve the requires-within-eventually issue, but we may try to tackle that with some custom AST in the future.Reviewer Checklist