-
Notifications
You must be signed in to change notification settings - Fork 107
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
Add argo lint on examples directory #1243
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1243 +/- ##
=====================================
Coverage 45.9% 45.9%
=====================================
Files 60 60
Lines 4096 4096
Branches 861 861
=====================================
Hits 1884 1884
Misses 2180 2180
Partials 32 32 ☔ View full report in Codecov by Sentry. |
@jeongukjae can you add it to the CI now? |
@sambhav not yet :( currently I see 55 lint errors if I run |
@jeongukjae can we run it but not fail on it? Or have some way to freeze the number of errors, so that it fails if it increases? |
fyi, there are three kind of lint errors now,
so, how about
if there's no strong reason to add this now? through this, I will assign myself to the issue (#1163), and fix this. |
Yeah, having tried a plain |
Signed-off-by: Ukjae Jeong <[email protected]>
Signed-off-by: Ukjae Jeong <[email protected]>
Signed-off-by: Ukjae Jeong <[email protected]>
@elliotgunton makes sense. Then remained issues are as follows:
I think those can be fixed. |
Pull Request Checklist
FixesAdd argo linting for examples #1163 (not fixing, need to more work to fix actual lint problems)Tests addednot needed hereDocumentation/examples addednot needed hereDescription of PR
This PR adds
make lint-argo
command to start work on issue #1163I will create additional fixes after this PR.
+ Since, the actual lint errors are not addressed yet, I didn't add it to CI checks or Contributing docs.