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

Allow globs in external codes setting #8176

Closed
wants to merge 1 commit into from
Closed

Conversation

zanieb
Copy link
Member

@zanieb zanieb commented Oct 24, 2023

Closes #8174

@zanieb zanieb added the configuration Related to settings and configuration label Oct 24, 2023
@zanieb
Copy link
Member Author

zanieb commented Oct 24, 2023

Alternatively, we could just match prefixes i.e. assume everything ends in *. This would be easier to use and might make more sense than globs for this purpose, however, it is technically a breaking change.

@github-actions
Copy link
Contributor

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

@charliermarsh
Copy link
Member

I think I'm partial to having the same logic we use for selectors elsewhere -- so, e.g., DOC matches DOC123, DOC1, etc. (Prefixes rather than globs, per your comment.) In what sense is it a breaking change? It feels strictly more lenient, so it wouldn't introduce any new violations.

@zanieb
Copy link
Member Author

zanieb commented Oct 24, 2023

I think I prefer prefixes too. I wish I hadn't bothered implementing it this way first haha

In what sense is it a breaking change? It feels strictly more lenient, so it wouldn't introduce any new violations.

Well we'll "no longer detect" unused noqa statements that would previously be have been raised, but no new violations is true. I think it's a breaking change in the most literal of senses, not something I'm worried about.

@zanieb zanieb closed this Oct 24, 2023
zanieb added a commit that referenced this pull request Oct 24, 2023
…8177)

Supersedes #8176
Closes #8174

## Test plan

Old snapshot contains the new / unmatched `V` code
New snapshot contains no `V` prefixed codes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
configuration Related to settings and configuration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Request: external also matching prefixes
2 participants