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

Support globbing patterns for source-roots #8290

Closed
alexey-pelykh opened this issue Feb 14, 2023 · 10 comments · Fixed by #8281
Closed

Support globbing patterns for source-roots #8290

alexey-pelykh opened this issue Feb 14, 2023 · 10 comments · Fixed by #8281
Labels
Needs specification 🔐 Accepted as a potential improvement, and needs to specify edge cases, message names, etc.
Milestone

Comments

@alexey-pelykh
Copy link
Contributor

Current problem

It's quite inconvenient having to specify every source root for complex multi-package projects like --source-roots src/package1,src/package2,...,src/packageN

Desired solution

For complex multi-package projects it would be nice to be able to specify source roots as --source-roots src/* instead of listing every one of them. IMHO, it's better to go with globbing patterns rather than with regexp patterns since those give better support for path-specific matching.

Additional context

No response

@alexey-pelykh alexey-pelykh added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Feb 14, 2023
@Pierre-Sassoulas
Copy link
Member

There's also --recursive=y, how would that work together ? Are those option mutually exclusive ?

@Pierre-Sassoulas Pierre-Sassoulas added Needs specification 🔐 Accepted as a potential improvement, and needs to specify edge cases, message names, etc. and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Feb 14, 2023
@alexey-pelykh
Copy link
Contributor Author

alexey-pelykh commented Feb 14, 2023

@Pierre-Sassoulas that's different story since this one is only about values passed to source-roots argument. Yet getting back to what you're talking about (the input files specification) - adding globbing support there is also a good idea and it would not interfere with recursive, but would supplement it I guess. Yet --recursive=y src/* is the same as --recursive=y src in the end result context while --recursive=y packages/*/src is not the same as --recursive=y packages

To achieve globbing patterns support for input specification, _config_initialization needs to be enhanced to support globbing, which should be transparent non-breakable change without any interference with --recursive=y

@Pierre-Sassoulas
Copy link
Member

My concern is that we have multiple ways to do the same things and it permits to do a lot of thing but also becoming really complicated at this point. I've been a maintainer for more than 5 years and I'm unclear on what should be done if pre-commit or find is not feeding file directly to pylint, this can't be a good sign.

I added the need specification label so we think about the option related to file discovery and try to make them work together and be consistent. In #5701 the consensus was that the default should be "current directory".

But should we make recursive=y if there's no __init__.py in the current directory ? source-root = src if there's an src directory ? Just fail and let the users fix their conf... ?

@alexey-pelykh
Copy link
Contributor Author

I don't really see a link between the concern which is valid yet outside of the scope of this issue. This issue only relates to enhancing what can be specified via source-roots and not altering any behavior.

I truly think that the concern you've raised worth a separate discussion.

@alexey-pelykh
Copy link
Contributor Author

That said, let's open a new RFC to discuss the concern of input discovery. Yet I should note that source-roots does not affect that since it only addresses "how to interpret the discovered structure"

@alexey-pelykh
Copy link
Contributor Author

@Pierre-Sassoulas I don't think this deserves 3.0.0 milestone really since it does not constitute a breaking change

@Pierre-Sassoulas
Copy link
Member

Yeah sorry, it's not a breaking change issue, it's just that I have too much on my plate and I don't anticipate to be able to deal (=discuss specs) before releasing 2.17.0.

@alexey-pelykh
Copy link
Contributor Author

Even taking into account that the original feature of source roots (to which this one is an improvement) was introduced at most a week ago?

@Pierre-Sassoulas
Copy link
Member

If another maintainer is available for discussion I'll trust their judgement on this.

@DanielNoord
Copy link
Collaborator

Sorry, my mistake! I agree that this would be a nice change!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs specification 🔐 Accepted as a potential improvement, and needs to specify edge cases, message names, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants