Skip to content

Allow globs in external codes setting#8176

Closed
zanieb wants to merge 1 commit intomainfrom
zanie/external-glob
Closed

Allow globs in external codes setting#8176
zanieb wants to merge 1 commit intomainfrom
zanie/external-glob

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