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

Update .pre-commit-config.yaml #286

Closed
wants to merge 7 commits into from
Closed

Update .pre-commit-config.yaml #286

wants to merge 7 commits into from

Conversation

ro4i7
Copy link

@ro4i7 ro4i7 commented Oct 1, 2023

Solving issue #275
Please check @kantord , i have used yamllint
i have done it under hacktoberfest2023

@kantord
Copy link
Owner

kantord commented Oct 2, 2023

Hi, looks like the lint tasks are still failing

- --verbose
- --ignore=tests/conftest.py

- repo: https://github.com/adrienverge/yamllint
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep in mind we already have this hook: https://github.com/jumanjihouse/pre-commit-hook-yamlfmt

could it cause a problem? I mean, will the 2 diffferent yaml linters/formatters interfere with each other? If both are actually needed, then could it be done in such a way that the second one will only care about the line length (by ignoring all other rules/formatting?)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes u r right, I have configured one of the linter to ignore line length, by making yamlfmt to only check line length

@stroncod
Copy link

stroncod commented Oct 2, 2023

Per docs in yamlfmt:

yamlfmt only works with valid YAML files, so I recommend to use yamllint and yamlfmt together.

So yamllint won't affect the behavior. Then to run yamlfmt to only check line-width is with --width that's why there is an error in the lint check. Default is 150 so technically is already doing formatting for YAML files, do you want that lower? It seems that's the only modification is need. @ro4i7 go ahead an finish the PR for your Hacktoberfest.

@ro4i7
Copy link
Author

ro4i7 commented Oct 3, 2023

Per docs in yamlfmt:

yamlfmt only works with valid YAML files, so I recommend to use yamllint and yamlfmt together.

So yamllint won't affect the behavior. Then to run yamlfmt to only check line-width is with --width that's why there is an error in the lint check. Default is 150 so technically is already doing formatting for YAML files, do you want that lower? It seems that's the only modification is need. @ro4i7 go ahead an finish the PR for your Hacktoberfest.

@stroncod ,thanks i have set the width. please check.

@kantord
Copy link
Owner

kantord commented Oct 3, 2023

let me see

Copy link
Owner

@kantord kantord left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it's giving these errors

usage: yamlfmt [-h] [-m MAPPING] [-s SEQUENCE] [-o OFFSET] [-c] [-w WIDTH]
               [-p] [--implicit_start | --explicit_start]
               [--implicit_end | --explicit_end] [-n]
               [FILE_NAME ...]
yamlfmt: error: unrecognized arguments: --line-length=80
usage: yamlfmt [-h] [-m MAPPING] [-s SEQUENCE] [-o OFFSET] [-c] [-w WIDTH]
               [-p] [--implicit_start | --explicit_start]
               [--implicit_end | --explicit_end] [-n]
               [FILE_NAME ...]
yamlfmt: error: unrecognized arguments: --line-length=80

remove print statements..................................................Passed
yamllint.................................................................Failed
- hook id: yamllint
- exit code: 2

usage: yamllint [-h] [-] [-c CONFIG_FILE | -d CONFIG_DATA]
                [-f {parsable,standard,colored,github,auto}] [-s]
                [--no-warnings] [-v]
                [FILE_OR_DIR ...]
yamllint: error: unrecognized arguments: --width=[80](https://github.com/kantord/SeaGOAT/actions/runs/6388169149/job/17344055464?pr=286#step:6:81)
usage: yamllint [-h] [-] [-c CONFIG_FILE | -d CONFIG_DATA]
                [-f {parsable,standard,colored,github,auto}] [-s]
                [--no-warnings] [-v]
                [FILE_OR_DIR ...]
yamllint: error: unrecognized arguments: --width=80

btw you can run it locally using pre-commit run --all-files that should also make sure that it passes on the CI

@ro4i7
Copy link
Author

ro4i7 commented Oct 3, 2023

Now it's giving these errors

usage: yamlfmt [-h] [-m MAPPING] [-s SEQUENCE] [-o OFFSET] [-c] [-w WIDTH]
               [-p] [--implicit_start | --explicit_start]
               [--implicit_end | --explicit_end] [-n]
               [FILE_NAME ...]
yamlfmt: error: unrecognized arguments: --line-length=80
usage: yamlfmt [-h] [-m MAPPING] [-s SEQUENCE] [-o OFFSET] [-c] [-w WIDTH]
               [-p] [--implicit_start | --explicit_start]
               [--implicit_end | --explicit_end] [-n]
               [FILE_NAME ...]
yamlfmt: error: unrecognized arguments: --line-length=80

remove print statements..................................................Passed
yamllint.................................................................Failed
- hook id: yamllint
- exit code: 2

usage: yamllint [-h] [-] [-c CONFIG_FILE | -d CONFIG_DATA]
                [-f {parsable,standard,colored,github,auto}] [-s]
                [--no-warnings] [-v]
                [FILE_OR_DIR ...]
yamllint: error: unrecognized arguments: --width=[80](https://github.com/kantord/SeaGOAT/actions/runs/6388169149/job/17344055464?pr=286#step:6:81)
usage: yamllint [-h] [-] [-c CONFIG_FILE | -d CONFIG_DATA]
                [-f {parsable,standard,colored,github,auto}] [-s]
                [--no-warnings] [-v]
                [FILE_OR_DIR ...]
yamllint: error: unrecognized arguments: --width=80

btw you can run it locally using pre-commit run --all-files that should also make sure that it passes on the CI

it used --width 80 format, in both yamlint and yamlfmt,

@ro4i7
Copy link
Author

ro4i7 commented Oct 3, 2023

Now it's giving these errors

usage: yamlfmt [-h] [-m MAPPING] [-s SEQUENCE] [-o OFFSET] [-c] [-w WIDTH]
               [-p] [--implicit_start | --explicit_start]
               [--implicit_end | --explicit_end] [-n]
               [FILE_NAME ...]
yamlfmt: error: unrecognized arguments: --line-length=80
usage: yamlfmt [-h] [-m MAPPING] [-s SEQUENCE] [-o OFFSET] [-c] [-w WIDTH]
               [-p] [--implicit_start | --explicit_start]
               [--implicit_end | --explicit_end] [-n]
               [FILE_NAME ...]
yamlfmt: error: unrecognized arguments: --line-length=80

remove print statements..................................................Passed
yamllint.................................................................Failed
- hook id: yamllint
- exit code: 2

usage: yamllint [-h] [-] [-c CONFIG_FILE | -d CONFIG_DATA]
                [-f {parsable,standard,colored,github,auto}] [-s]
                [--no-warnings] [-v]
                [FILE_OR_DIR ...]
yamllint: error: unrecognized arguments: --width=[80](https://github.com/kantord/SeaGOAT/actions/runs/6388169149/job/17344055464?pr=286#step:6:81)
usage: yamllint [-h] [-] [-c CONFIG_FILE | -d CONFIG_DATA]
                [-f {parsable,standard,colored,github,auto}] [-s]
                [--no-warnings] [-v]
                [FILE_OR_DIR ...]
yamllint: error: unrecognized arguments: --width=80

btw you can run it locally using pre-commit run --all-files that should also make sure that it passes on the CI

it used --width 80 format, in both yamlint and yamlfmt,

it used --width format , but still it giving linting error in yamlint

@stroncod
Copy link

stroncod commented Oct 3, 2023

That's because yamlint does not have the --width, yamlint just check if the YAML files are correct and valid following YAML rules.

Now it's giving these errors

usage: yamlfmt [-h] [-m MAPPING] [-s SEQUENCE] [-o OFFSET] [-c] [-w WIDTH]
               [-p] [--implicit_start | --explicit_start]
               [--implicit_end | --explicit_end] [-n]
               [FILE_NAME ...]
yamlfmt: error: unrecognized arguments: --line-length=80
usage: yamlfmt [-h] [-m MAPPING] [-s SEQUENCE] [-o OFFSET] [-c] [-w WIDTH]
               [-p] [--implicit_start | --explicit_start]
               [--implicit_end | --explicit_end] [-n]
               [FILE_NAME ...]
yamlfmt: error: unrecognized arguments: --line-length=80

remove print statements..................................................Passed
yamllint.................................................................Failed
- hook id: yamllint
- exit code: 2

usage: yamllint [-h] [-] [-c CONFIG_FILE | -d CONFIG_DATA]
                [-f {parsable,standard,colored,github,auto}] [-s]
                [--no-warnings] [-v]
                [FILE_OR_DIR ...]
yamllint: error: unrecognized arguments: --width=[80](https://github.com/kantord/SeaGOAT/actions/runs/6388169149/job/17344055464?pr=286#step:6:81)
usage: yamllint [-h] [-] [-c CONFIG_FILE | -d CONFIG_DATA]
                [-f {parsable,standard,colored,github,auto}] [-s]
                [--no-warnings] [-v]
                [FILE_OR_DIR ...]
yamllint: error: unrecognized arguments: --width=80

btw you can run it locally using pre-commit run --all-files that should also make sure that it passes on the CI

it used --width 80 format, in both yamlint and yamlfmt,

it used --width format , but still it giving linting error in yamlint

Try using the flag on yamlfmt and not on yamlint.

@ro4i7
Copy link
Author

ro4i7 commented Oct 3, 2023

Try using the flag on yamlfmt and not on yamlint.

ohk i have done it, please check

@kantord
Copy link
Owner

kantord commented Oct 7, 2023

Try using the flag on yamlfmt and not on yamlint.

ohk i have done it, please check

hi, it's still failing from what I can see

@ro4i7
Copy link
Author

ro4i7 commented Oct 7, 2023

Try using the flag on yamlfmt and not on yamlint.

ohk i have done it, please check

hi, it's still failing from what I can see

Hello, Yes it's still failing. Any suggestions?

@stroncod
Copy link

stroncod commented Oct 8, 2023

Try using the flag on yamlfmt and not on yamlint.

ohk i have done it, please check

hi, it's still failing from what I can see

Hello, Yes it's still failing. Any suggestions?

It fails because yamlfmt does not modify the files, only notifies you. You can run a script to solve that after yamlfmt tells you which files you need

@kantord
Copy link
Owner

kantord commented Oct 9, 2023

Also now the branch needs to be rebased it seems

@kantord kantord closed this Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants