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

CI: fix pre-commit warnings/errors in linting #3246

Merged
merged 1 commit into from
Nov 22, 2023

Conversation

nilason
Copy link
Contributor

@nilason nilason commented Nov 18, 2023

Address pre-commit warnings and fixes. A majority of the issues originate from CI bot PRs, but not all.
This emphasises the need to explore the introduction of pre-commit.ci.

@nilason nilason added the add to git-blame-ignore-revs Once merged, add to .git-blame-ignore-revs label Nov 19, 2023
@nilason
Copy link
Contributor Author

nilason commented Nov 20, 2023

If there are no objections I'll take the liberty to merge this "whitespace" fix.

Comment on lines +117 to +120
echo "::warning file=.github/workflows/python-code-quality.yml,line=116,col=42,endColumn=48::\
Temporarily downgraded pytest-pylint to allow merging other PRs. The errors reported\
with a newer version seem legitimite and should be fixed (2023-10-18,\
see https://github.com/OSGeo/grass/pull/3205)"
Copy link
Member

Choose a reason for hiding this comment

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

It breaks the output of the warning, but that warning was written by me to intentionally mentioning that the test failures must be fixed one day to upgrade the package version.

Copy link
Member

Choose a reason for hiding this comment

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

It can stay as is!

Copy link
Member

@echoix echoix left a comment

Choose a reason for hiding this comment

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

I don't have any deciding powers, but I still approve these changes :)

@nilason
Copy link
Contributor Author

nilason commented Nov 20, 2023

The warning is there:

Successfully installed pytest-pylint-0.19.0
Warning:   Temporarily downgraded pytest-pylint to allow merging other PRs. The errors reported  with a newer version seem legitimite and should be fixed (2023-10-18,  see https://github.com/OSGeo/grass/pull/3205)
============================= test session starts ==============================

with colours and all.

@echoix
Copy link
Member

echoix commented Nov 20, 2023

The warning is there:

Successfully installed pytest-pylint-0.19.0
Warning:   Temporarily downgraded pytest-pylint to allow merging other PRs. The errors reported  with a newer version seem legitimite and should be fixed (2023-10-18,  see https://github.com/OSGeo/grass/pull/3205)
============================= test session starts ==============================

with colours and all.

It is the extra spaces in between some words that come from the new indentation. But either way, the goal is to have it removed some day, so it doesn't matter at all.

@nilason
Copy link
Contributor Author

nilason commented Nov 20, 2023

It is the extra spaces in between some words that come from the new indentation. But either way, the goal is to have it removed some day, so it doesn't matter at all.

Yes, there is one extra space after warning. But the warning is issued correctly. Multiline strings in yaml is a PITA.

@nilason
Copy link
Contributor Author

nilason commented Nov 21, 2023

I will update this after #3254 is merged.

@neteler neteler added this to the 8.4.0 milestone Nov 21, 2023
@nilason nilason force-pushed the fix_precommit_errors branch from 343c16f to dbdc6df Compare November 22, 2023 12:05
@nilason nilason self-assigned this Nov 22, 2023
@nilason nilason merged commit ef229b7 into OSGeo:main Nov 22, 2023
19 checks passed
@nilason nilason deleted the fix_precommit_errors branch November 22, 2023 14:14
HuidaeCho pushed a commit to HuidaeCho/grass that referenced this pull request Jan 9, 2024
@neteler neteler changed the title lint: fix pre-commit warnings/errors CI: fix pre-commit warnings/errors in linting Jun 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add to git-blame-ignore-revs Once merged, add to .git-blame-ignore-revs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants