-
Notifications
You must be signed in to change notification settings - Fork 139
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 ruff #620
Add ruff #620
Conversation
91754ff
to
5981801
Compare
The test failures seemed unrelated. |
function getFixParams(dir) { | ||
return { | ||
// Expected output of the linting function | ||
cmdOutput: { | ||
status: 1, | ||
stdout: "", | ||
}, | ||
// Expected output of the parsing function | ||
lintResult: { | ||
isSuccess: false, | ||
warning: [], | ||
error: [], | ||
}, | ||
}; | ||
} |
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.
@pickfire is it possible that the expected values in the ruff
+ fix
are wrong?
I ran the tests on my machine for ruff
and without the following change the tests were failing:
function getFixParams(dir) {
return {
// Expected output of the linting function
cmdOutput: {
status: 0,
stdout: "",
},
// Expected output of the parsing function
lintResult: {
isSuccess: true,
warning: [],
error: [],
},
};
}
the thing is the option --fix-only
at
lint-action/src/linters/ruff.js
Lines 52 to 53 in 5981801
const fixArg = fix ? "--fix-only" : ""; | |
return run(`${prefix} ruff check --quiet ${fixArg} ${args} .`, { |
ruff
doesn't report the violations:
--fix-only
Fix any fixable lint violations, but don't report on leftover violations.
Here is my context:
$ uname -s -r -p -o
Linux 5.19.0-41-generic x86_64 GNU/Linux
$ node --version
v16.20.0
$ jest --version
29.3.1
$ python --version
Python 3.10.11
$ pip freeze | grep ruff
ruff==0.0.267
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.
Haven't looked at this PR closely, but FYI we recently changed --fix-only
to always return a code of 0, unless you provide --exit-non-zero-on-fix
, in which case, it'll exit 1 if it fixes any violations.
Relevant link to the breaking change list: https://github.com/charliermarsh/ruff/blob/main/BREAKING_CHANGES.md#--fix-only-now-exits-with-a-zero-exit-code-unless---exit-non-zero-on-fix-is-specified-4146
I don't get why is black linter failing in the tests. |
|
fix: black & mypy
Co-authored-by: Felipe Valverde <[email protected]>
@fvalverd, tests still seem to be failing |
There are 3 failures:
|
I don't know if it makes anything easier, but https://docs.astral.sh/ruff/settings/#output-format $ ruff check --output-format github .
::error title=Ruff (C408),file=/home/luzfcb/projects/labcodes/pythondotorg/events/tests/test_utils.py,line=36,col=17,endLine=36,endColumn=60::events/tests/test_utils.py:36:17: C408 Unnecessary `dict` call (rewrite as a literal) $ ruff check --output-format json .
[
{
"cell": null,
"code": "C408",
"end_location": {
"column": 60,
"row": 36
},
"filename": "/home/luzfcb/projects/labcodes/pythondotorg/events/tests/test_utils.py",
"fix": null,
"location": {
"column": 17,
"row": 36
},
"message": "Unnecessary `dict` call (rewrite as a literal)",
"noqa_row": 36,
"url": "https://docs.astral.sh/ruff/rules/unnecessary-collection-call"
}
] |
Any progress on this PR? |
@BroFromSpace I no longer need this, I guess you can continue working on it. Will close this. |
Fix #603