-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add initial warnings regarding license-files
glob patterns.
#4838
Conversation
44ae921
to
922b38d
Compare
I also started working on this before I saw you've opened this PR. As I chose a slightly different approach, I finished it so we could compare them better. See #4841 I explicitly decided to check that each glob pattern match at least one file as I think this is by far the most useful check to have. It's a fallback to make sure files actually get matched. I don't think project templates are a big deal. They either didn't add it in the first place as setuptools provided good defaults early on or used a value specific to their configuration. Why should they have added multiple patterns if they didn't use these files? |
922b38d
to
095990f
Compare
Thank you very much @cdce8p, I tried to incorporate the validations from #4841 into this one according to the discussion in #4841 (comment). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Adding the enforce_match
argument makes a lot of sense. Will go ahead and close my PR then.
Thank you very much. Waiting feedback from https://discuss.python.org/t/pep-639-round-3-improving-license-clarity-with-better-package-metadata/53020/174. |
Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
095990f
to
c31ebdc
Compare
Interestingly after I merged #4842 and rebased the PR, |
Not sure if you'll even get a response. Just checked Hatchling only seems to check that the pattern is a str and uses Pdm-backend doesn't even do glob matching for user supplied values 🤷🏻♂️ Poetry hasn't added support yet AFAIK and flit isn't a good comparison since I contributed the pattern validation there. -- |
Than you very much @cdce8p. I updated the PR based on your feedback. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. As mentioned earlier, I'm not sure there will be feedback on this in the forum. It's probably a case for iterative improvements. See what works and where the pain points are.
Ref: #4829 (comment)
This by no means introduces an exhaustive check, just the low hanging fruits.
I intentionally do not analyse each pattern individually here to avoid wreaking too much havoc (imagine e.g. project templates that adds a very general catch all list of patterns in
setup.cfg
).Regarding the
glob
patterns, I am keeping loose for the time being, would like to avoid adding a regex for each pattern if possible, but open to it.../cc @cdce8p
Pull Request Checklist
newsfragments/
.(See documentation for details)