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

Ruff run format command #3295

Open
RomeoAva opened this issue Jan 17, 2024 · 18 comments · May be fixed by #4329
Open

Ruff run format command #3295

RomeoAva opened this issue Jan 17, 2024 · 18 comments · May be fixed by #4329
Labels
question Further information is requested

Comments

@RomeoAva
Copy link

Hello,
I am trying to use Ruff through this Github action. My issue is when I use the command ruff check file.py it will only alert me on bugs such as imports not used etc, but doesn't alert me on formatting errors. To get those errors I use the command ruff format --check file.py which would give me:

Would reformat: app/file.py
1 file would be reformatted

My issue is that the megalinter ruff linter only seems to allow the check command. I tried to tweak it inside the mega-linter.yml config but not luck:

  PYTHON_RUFF_COMMAND_REMOVE_ARGUMENTS: 'check'
  PYTHON_RUFF_ARGUMENTS: 'format --check'

https://megalinter.io/latest/descriptors/python_ruff/#ruff-documentation

It works with the official Github ruff-action with this workflow but I would prefer to use the one with megalinter since it has more overall features

name: Ruff
on: [pull_request]
jobs:
  ruff:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
      - uses: chartboost/ruff-action@v1
        with:
          src: "./app"
          args: format --check

Any ideas? Thank you for your help

@RomeoAva RomeoAva added the question Further information is requested label Jan 17, 2024
@nvuillam
Copy link
Member

nvuillam commented Jan 20, 2024

@RomeoAva please can you try with:

  PYTHON_RUFF_COMMAND_REMOVE_ARGUMENTS: 'check'
  PYTHON_RUFF_ARGUMENTS: ['format','--check']
  LOG_LEVEL: DEBUG

And see what is the command line built by MegaLinter in the logs ?

@RomeoAva
Copy link
Author

@RomeoAva please can you try with:

  PYTHON_RUFF_COMMAND_REMOVE_ARGUMENTS: 'check'
  PYTHON_RUFF_ARGUMENTS: ['format','--check']
  LOG_LEVEL: DEBUG

And see what is the command line built by MegaLinter in the logs ?

Hi @nvuillam, when using PYTHON_RUFF_ARGUMENTS: ['format','--check'] I face the following error:

The workflow is not valid. .github/workflows/mega-linter.yml (Line: 27, Col: 26): A sequence was not expected

I believe this is due to the brackets not being allowed here.

And when running it with the following:

  PYTHON_RUFF_COMMAND_REMOVE_ARGUMENTS: 'check'
  PYTHON_RUFF_ARGUMENTS: 'format --check'

I get this error:

❌ Linted [PYTHON] files with [ruff]: Found 1 error(s) - (0.02s) (expand for details)
  - Using [ruff v0.1.14] https://megalinter.io/beta/descriptors/python_ruff
  - MegaLinter key: [PYTHON_RUFF]
  - Rules config: [.ruff.toml]
  - Number of files analyzed: [11]
  --Error detail:
  error: unexpected argument '--check' found
  
    tip: to pass '--check' as a value, use '-- --check'
  
  Usage: ruff check <FILES|--fix|--no-fix|--unsafe-fixes|--no-unsafe-fixes|--show-source|--no-show-source|--show-fixes|--no-show-fixes|--diff|--watch|--fix-only|--no-fix-only|--ignore-noqa|--output-format <OUTPUT_FORMAT>|--output-file <OUTPUT_FILE>|--target-version <TARGET_VERSION>|--preview|--no-preview|--config <CONFIG>|--select <RULE_CODE>|--ignore <RULE_CODE>|--extend-select <RULE_CODE>|--extend-ignore <RULE_CODE>|--per-file-ignores <PER_FILE_IGNORES>|--extend-per-file-ignores <EXTEND_PER_FILE_IGNORES>|--exclude <FILE_PATTERN>|--extend-exclude <FILE_PATTERN>|--fixable <RULE_CODE>|--unfixable <RULE_CODE>|--extend-fixable <RULE_CODE>|--extend-unfixable <RULE_CODE>|--respect-gitignore|--no-respect-gitignore|--force-exclude|--no-force-exclude|--line-length <LINE_LENGTH>|--dummy-variable-rgx <DUMMY_VARIABLE_RGX>|--no-cache|--isolated|--cache-dir <CACHE_DIR>|--stdin-filename <STDIN_FILENAME>|--extension <EXTENSION>|--exit-zero|--exit-non-zero-on-fix|--statistics|--add-noqa|--show-files|--show-settings|--ecosystem-ci>

@nvuillam
Copy link
Member

Would this work ?

PYTHON_RUFF_ARGUMENTS:
- format
- --check

@echoix
Copy link
Collaborator

echoix commented Jan 27, 2024

From what I understand, formatting in ruff is more like a second tool, like an (almost) drop in replacement for black, and the linter is a replacement for all other linters. So it's quite possible we'd have to call it like a linter, and a second time just like we call black, for the formatting part. The check flag is like black's check flag, where it will only show if it would change the file, but not do the changes.

@nvuillam
Copy link
Member

So maybe we should declare a second linter "PYTHON_RUFF_FORMATTER" ?

@ipmb
Copy link

ipmb commented Feb 13, 2024

That's correct @echoix

Ruff has a linter that is closer to what Flake8 does That is run with ruff check and ruff check --fix

It recently released a formatter that can replace Black. That is run with ruff format --check and ruff format

Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Mar 15, 2024
@echoix
Copy link
Collaborator

echoix commented Mar 15, 2024

Bump

@github-actions github-actions bot removed the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Mar 16, 2024
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Apr 15, 2024
@echoix echoix removed the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Apr 15, 2024
@echoix
Copy link
Collaborator

echoix commented Apr 16, 2024

@Skitionek

Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label May 17, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 31, 2024
@echoix echoix reopened this May 31, 2024
@github-actions github-actions bot removed the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Jun 1, 2024
@paulostradioti
Copy link

Hi guys,
Are there any updates on this? It'd be very useful to also being able to run the format check as well.

@JobaDiniz
Copy link

Until this is not made officially available, is there any workaround @RomeoAva that you are using?

@RomeoAva RomeoAva reopened this Jun 18, 2024
@RomeoAva
Copy link
Author

RomeoAva commented Jun 18, 2024

Hi all, I didn't find a work around using this action unfortunately.
I ended up using this Ruff action with something like this:

name: Ruff
on:
  workflow_call:
jobs:
  ruff:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
      - uses: chartboost/ruff-action@v1
        with:
          src: "./app"
          args: format --check

@echoix
Copy link
Collaborator

echoix commented Jun 24, 2024

@nvuillam and maybe even @TommyE123 Do you have an idea on the approach to use for implementing ruff format?

ruff was initially only a linter. Now it has fixes too.
Then ruff added another command, ruff format, that acts similar to black, to format files. Black and ruff, while very similar, can't really be used together, because of known and justified differences. As I tried again today, one will make changes that the other will also change, and running the first again will still revert some changes. As such, it is either one or the other.

For me, its clear that the linter key will be PYTHON_RUFF_FORMAT. What about the existing one, is it still PYTHON_RUFF? Or should it be PYTHON_RUFF_CHECK? That would be quite a breaking change.

There is also a list of ruff rules that contradict the ruff formatter, as listed here: https://docs.astral.sh/ruff/formatter/#conflicting-lint-rules. By default, ruff only enables a subset of the now 800+ rules, excluding the ones conflicting with the formatter. Is there anything for us to do here?

@nvuillam
Copy link
Member

nvuillam commented Jun 24, 2024

I think that if a linter can format and fix, it should be declared twice:

  • once as a formatter
  • once as a linter that can fix

MegaLinter internal architeture is able to work with the pattern CHECK -> FIX , there is no room for CHECK -> FORMAT -> FIX ^^

And formatters are also non blocking by default

Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Jul 25, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Aug 8, 2024
@estahn
Copy link

estahn commented Nov 29, 2024

@nvuillam I agree. Adding a PYTHON_RUFF_FORMAT shouldn't be breaking the existing concepts.

Is this moving forward?

@nvuillam nvuillam reopened this Nov 30, 2024
nvuillam added a commit that referenced this issue Nov 30, 2024
Fixes #3295
@nvuillam nvuillam linked a pull request Nov 30, 2024 that will close this issue
@github-actions github-actions bot removed the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants