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

tools: disable unneeded rule ignoring in Python linting #56429

Merged
merged 1 commit into from
Jan 3, 2025

Conversation

Trott
Copy link
Member

@Trott Trott commented Jan 1, 2025

Removing PLC1901 and RUF100 from the list of Python lint rules to ignore does not result in any errors. Keeping the ignore list as short as possible seems like a good idea, so this change removes them from the ignore list.

Removing PLC1901 and RUF100 from the list of Python lint rules to ignore
does not result in any errors. Keeping the ignore list as short as
possible seems like a good idea, so this change removes them from the
ignore list.
@Trott Trott requested a review from cclauss January 1, 2025 23:03
@Trott
Copy link
Member Author

Trott commented Jan 1, 2025

@nodejs/python

@Trott Trott added python PRs and issues that require attention from people who are familiar with Python. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jan 2, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 2, 2025
@nodejs-github-bot
Copy link
Collaborator

Copy link
Contributor

@cclauss cclauss left a comment

Choose a reason for hiding this comment

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

Nice!

@Trott Trott added the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 2, 2025
@cclauss
Copy link
Contributor

cclauss commented Jan 2, 2025

Merging is blocked
Commit message must match a given regex pattern: ...

It would be nice to have a precommit-like capability at the level of the GitHub UI that would complain before the PR was created instead of after the PR is approved.

The First commit message adheres to guidelines test passes yet Merging is blocked.

@legendecas
Copy link
Member

Merging is blocked
Commit message must match a given regex pattern: ...

It would be nice to have a precommit-like capability at the level of the GitHub UI that would complain before the PR was created instead of after the PR is approved.

The First commit message adheres to guidelines test passes yet Merging is blocked.

The "merge" operation in this repo has to be performed with commit-queue Add this label to land a pull request using GitHub Actions. or https://github.com/nodejs/node-core-utils/blob/main/docs/git-node.md#git-node-land. The merge button on the GitHub UI is banned.

@Trott
Copy link
Member Author

Trott commented Jan 2, 2025

Merging is blocked
Commit message must match a given regex pattern: ...

It would be nice to have a precommit-like capability at the level of the GitHub UI that would complain before the PR was created instead of after the PR is approved.

The First commit message adheres to guidelines test passes yet Merging is blocked.

I could be misremembering, but I think this is by design. The commit message format is correct but the commit queue hasn't yet added the appropriate metadata. But that happens automatically. Users are not supposed to supply it themselves. This way, it disables the Merge button. We don't want people using the button. We want commits landed through the commit queue. This PR currently has a commit-queue label and so will be landed automatically after it's been open for 48 hours (unless someone requests changes or a small number of other reasons).

If I'm mistaken, then sorry for the misinformation!

And if I'm correct but there is some way to improve the interface to make it less confusing, let's do that!

@cclauss
Copy link
Contributor

cclauss commented Jan 2, 2025

Commit message must match a given regex pattern: PR-URL: https://github\.com/(nodejs|nodejs-private)/(node|node-private)/pull/\d+

-->

Merge in this repo has to be performed with commit-queue Add this label to land a pull request using GitHub Actions. or https://github.com/nodejs/node-core-utils/blob/main/docs/git-node.md#git-node-land. The merge button on the GitHub UI is banned.

@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 3, 2025
@nodejs-github-bot nodejs-github-bot merged commit 804d41f into nodejs:main Jan 3, 2025
67 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 804d41f

@Trott Trott deleted the ignore-list branch January 3, 2025 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. python PRs and issues that require attention from people who are familiar with Python.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants