Skip to content

[Merged by Bors] - fix(lint-style.py): recognise all kinds of style exceptions#16297

Closed
grunweg wants to merge 1 commit intomasterfrom
MR-lint-style-recognise-all
Closed

[Merged by Bors] - fix(lint-style.py): recognise all kinds of style exceptions#16297
grunweg wants to merge 1 commit intomasterfrom
MR-lint-style-recognise-all

Conversation

@grunweg
Copy link
Copy Markdown
Contributor

@grunweg grunweg commented Aug 30, 2024

Currently, some style exceptions emitted by the lint-style.py script are not recognised by that same script. This can lead to confusing behaviour.
That said, all current exceptions should rather be fixed than excepted - so this is unlikely to be an issue in practice.


Open in Gitpod

@grunweg grunweg added easy < 20s of review time. See the lifecycle page for guidelines. t-linter Linter labels Aug 30, 2024
@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 30, 2024

PR summary 0c387bcf7a

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

No declarations were harmed in the making of this PR! 🐙

You can run this locally as follows
## summary with just the declaration names:
./scripts/declarations_diff.sh <optional_commit>

## more verbose report:
./scripts/declarations_diff.sh long <optional_commit>

The doc-module for script/declarations_diff.sh contains some details about this script.

@grunweg grunweg changed the title fix(lint-stype.py): recognise all kinds of style exceptions fix(lint-style.py): recognise all kinds of style exceptions Aug 30, 2024
@grunweg grunweg requested a review from adomani August 30, 2024 11:09
@leanprover-community-bot-assistant leanprover-community-bot-assistant added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Aug 30, 2024
Not recognising one is more confusing than any other behaviour.
In practice, after #15610 none of these should be needed in practice.
@grunweg grunweg force-pushed the MR-lint-style-recognise-all branch from 35b62b7 to 0c387bc Compare August 30, 2024 14:19
@grunweg grunweg removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Aug 30, 2024
@adomani
Copy link
Copy Markdown
Contributor

adomani commented Aug 30, 2024

Thanks!

maintainer merge

If I understand correctly, the general trend is to eliminate all the error codes that are maintained by the python/bash linters. However, this means that if for some reason one of these error codes creeps up again, the script will not be confused, right?

@github-actions
Copy link
Copy Markdown

🚀 Pull request has been placed on the maintainer queue by adomani.

@github-actions github-actions bot added the maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. label Aug 30, 2024
@grunweg
Copy link
Copy Markdown
Contributor Author

grunweg commented Aug 30, 2024

To clarify: all errors added are maintained by lint-style.py; I'm merely making it recognise those.

In the medium-term, I would propose removing style-exceptions.txt altogether, as all remaining text-based linters should not need to have permanent or transient exceptions committed. That can be a different PR, though.

@urkud
Copy link
Copy Markdown
Member

urkud commented Aug 31, 2024

Thanks! 🎉
bors merge

@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label Aug 31, 2024
mathlib-bors bot pushed a commit that referenced this pull request Aug 31, 2024
Currently, some style exceptions emitted by the `lint-style.py` script are not recognised by that same script. This can lead to confusing behaviour.
That said, all current exceptions should rather be fixed than excepted - so this is unlikely to be an issue in practice.
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Aug 31, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title fix(lint-style.py): recognise all kinds of style exceptions [Merged by Bors] - fix(lint-style.py): recognise all kinds of style exceptions Aug 31, 2024
@mathlib-bors mathlib-bors bot closed this Aug 31, 2024
@mathlib-bors mathlib-bors bot deleted the MR-lint-style-recognise-all branch August 31, 2024 23:21
bjoernkjoshanssen pushed a commit that referenced this pull request Sep 9, 2024
Currently, some style exceptions emitted by the `lint-style.py` script are not recognised by that same script. This can lead to confusing behaviour.
That said, all current exceptions should rather be fixed than excepted - so this is unlikely to be an issue in practice.
bjoernkjoshanssen pushed a commit that referenced this pull request Sep 9, 2024
Currently, some style exceptions emitted by the `lint-style.py` script are not recognised by that same script. This can lead to confusing behaviour.
That said, all current exceptions should rather be fixed than excepted - so this is unlikely to be an issue in practice.
bjoernkjoshanssen pushed a commit that referenced this pull request Sep 12, 2024
Currently, some style exceptions emitted by the `lint-style.py` script are not recognised by that same script. This can lead to confusing behaviour.
That said, all current exceptions should rather be fixed than excepted - so this is unlikely to be an issue in practice.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

easy < 20s of review time. See the lifecycle page for guidelines. maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. ready-to-merge This PR has been sent to bors. t-linter Linter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants