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 opt-out of find_files behavior. (issue 561) #851

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

daveware-nv
Copy link

@daveware-nv daveware-nv commented May 23, 2023

Changes it so find_files is only enabled with config parameter enable_find_files is set to true, otherwise will always return empty list.

Adds config argument enable_find_files that when set to false allows find_files behavior to be disabled.

Addresses #561

Changes it so find_files is only enabled with config parameter
`enable_find_files` is set to true, otherwise will always return empty
list.
@daveware-nv daveware-nv force-pushed the make_find_file_opt_in_main branch from 1772b25 to 819fbad Compare May 23, 2023 23:22
Copy link
Contributor

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

for legacy support this has to be opt-in

im researching ways to enable this better by adding a new sdist subcommand instead of obtaining a unrelated possibly miss-configured config object for just the file listing

@daveware-nv
Copy link
Author

OK, I've switched the default to true.

@ljames8
Copy link

ljames8 commented Jul 11, 2023

isn't this PR good to be merged now the requested changes have been made ?

@RonnyPfannschmidt
Copy link
Contributor

cant merge this yet as opting out of the file finders possibly opts into version finding + there is a bug

@Lenbok
Copy link

Lenbok commented Oct 12, 2023

@RonnyPfannschmidt Do you have more information, e.g. what is the bug that is preventing this being merged?

@webknjaz
Copy link
Member

@daveware-nv looks like the PR title needs to be fixed to reflect the post-review changes. Could you update it?

@daveware-nv daveware-nv changed the title Make find_files behavior opt-in. (issue 561) Allow opt-out of find_files behavior. (issue 561) Feb 26, 2024
@RonnyPfannschmidt
Copy link
Contributor

currently there are many merge conflicts - im unable to merge as is

in the next major release i want to switch from file finders to custom setuptools subcommands as currently file finder opt out requires setuptools_scm "opt-in"

@jaraco
Copy link
Member

jaraco commented Aug 8, 2024

cant merge this yet as opting out of the file finders possibly opts into version finding + there is a bug

In #364 (comment), I advocated for there being a separate boolean to enable (or disable) version inference. Probably that needs to be implemented (first) so people can opt out of versions inference as well. Either that, or the tool.setuptools_scm needs to be deprecated and superseded by a new name that could have different defaults (and doesn't use the section header to imply enablement of a feature).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants