Skip to content
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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add ruff #620

wants to merge 5 commits into from

Conversation

pickfire
Copy link

Fix #603

README.md Outdated Show resolved Hide resolved
@pickfire pickfire force-pushed the ruff branch 4 times, most recently from 91754ff to 5981801 Compare March 21, 2023 15:32
@pickfire
Copy link
Author

pickfire commented Mar 21, 2023

The test failures seemed unrelated.

Comment on lines +51 to +65
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: [],
},
};
}

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

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

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

@pickfire
Copy link
Author

pickfire commented Jun 6, 2023

I don't get why is black linter failing in the tests.

@fvalverd
Copy link

fvalverd commented Sep 3, 2023

I don't get why is black linter failing in the tests.

mypy and black latest version are failing because the output format changed, I provided you a PR with the necessary changes to make them pass in here https://github.com/pickfire/lint-action/pull/1/files

Co-authored-by: Felipe Valverde <407122+fvalverd@users.noreply.github.com>
@tylerlaprade
Copy link

@fvalverd, tests still seem to be failing

@fvalverd
Copy link

fvalverd commented Oct 9, 2023

There are 3 failures:

  • macos: that is swift, nothing to do with this PR or Python in general
  • ubuntu: that is also swift, it has nothing to do with this PR or Python in general
  • windows: those are related to ruff and black, probably the syntax of the output is the problem, but I don't have access to that OS to corroborate the problem

@luzfcb
Copy link

luzfcb commented Oct 31, 2023

I don't know if it makes anything easier, but ruff currently supports some output formats, such as github and json

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"
  }
]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for ruff
6 participants