Skip to content

[Merged by Bors] - chore: remove sed-based hashCommand linter#13142

Closed
grunweg wants to merge 2 commits intomasterfrom
MR-remove-old-hash-linter
Closed

[Merged by Bors] - chore: remove sed-based hashCommand linter#13142
grunweg wants to merge 2 commits intomasterfrom
MR-remove-old-hash-linter

Conversation

@grunweg
Copy link
Copy Markdown
Contributor

@grunweg grunweg commented May 23, 2024

The linter was rewritten in Lean in #11019. This removes one separate workflow step.


Open in Gitpod

@grunweg grunweg added awaiting-review CI Modifies the continuous integration setup or other automation labels May 23, 2024
@grunweg grunweg requested a review from adomani May 23, 2024 11:25
@grunweg grunweg added blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) t-linter Linter labels May 23, 2024
@ghost ghost removed the blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) label May 24, 2024
@ghost
Copy link
Copy Markdown

ghost commented May 24, 2024

This PR/issue depends on:

@adomani
Copy link
Copy Markdown
Contributor

adomani commented May 24, 2024

This looks good to me, of course!

If you merge master, the diff will become much smaller, now that the linter is in mathlib!

@grunweg grunweg force-pushed the MR-remove-old-hash-linter branch from 05bb8ed to b861ad8 Compare May 24, 2024 07:49
@grunweg
Copy link
Copy Markdown
Contributor Author

grunweg commented May 24, 2024

I rebased master (and fixed a typo in the HashCommandLinter's docstring - which I remembered too late).

@adomani
Copy link
Copy Markdown
Contributor

adomani commented May 24, 2024

Thanks for fixing the typo!

I am happy with removing the CI step, although I wonder whether waiting for a couple of days to see them both working at the same time is a good idea.

@grunweg
Copy link
Copy Markdown
Contributor Author

grunweg commented May 24, 2024

I'm also happy to wait for a few days - to me, it's just unclear if we would ever get such a signal (since the linter errors locally already).

@adomani
Copy link
Copy Markdown
Contributor

adomani commented May 26, 2024

I think that this is indeed an improvement.

maintainer merge

@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 May 26, 2024
@jcommelin
Copy link
Copy Markdown
Member

Thanks 🎉

bors merge

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review labels May 27, 2024
mathlib-bors bot pushed a commit that referenced this pull request May 27, 2024
The linter was rewritten in Lean in #11019. This removes one separate workflow step.
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented May 27, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore: remove sed-based hashCommand linter [Merged by Bors] - chore: remove sed-based hashCommand linter May 27, 2024
@mathlib-bors mathlib-bors bot closed this May 27, 2024
@mathlib-bors mathlib-bors bot deleted the MR-remove-old-hash-linter branch May 27, 2024 07:04
callesonne pushed a commit that referenced this pull request Jun 4, 2024
The linter was rewritten in Lean in #11019. This removes one separate workflow step.
js2357 pushed a commit that referenced this pull request Jun 18, 2024
The linter was rewritten in Lean in #11019. This removes one separate workflow step.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Modifies the continuous integration setup or other automation 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.

3 participants