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

Set the fromfile option arg's default to True #7513

Merged
merged 4 commits into from Apr 10, 2019

Conversation

Projects
None yet
4 participants
@benjyw
Copy link
Contributor

commented Apr 6, 2019

There's no reason not to support this for all options.
If this goes smoothly a future change will deprecate the fromfile arg.

benjyw added some commits Apr 6, 2019

Set the fromfile option arg's default to be True.
There's no reason not to support this for all options.
If this goes smoothly a future change will deprecate
the fromfile arg.

@benjyw benjyw requested review from stuhood and Eric-Arellano Apr 6, 2019

@benjyw

This comment has been minimized.

Copy link
Contributor Author

commented Apr 6, 2019

See #7507

@benjyw

This comment has been minimized.

Copy link
Contributor Author

commented Apr 6, 2019

Note that the commits can be meaningfully reviewed separately, if convenient.

@illicitonion
Copy link
Contributor

left a comment

Hurrah!

if val_str.startswith('@@'): # Support a literal @ for fromfile values via @@.
return val_str[1:]
def expand(val_or_str):
if (kwargs.get('fromfile', True) and val_or_str and

This comment has been minimized.

Copy link
@illicitonion

illicitonion Apr 8, 2019

Contributor

Does kwargs ever contain 'fromfile' here or is this using the default 100% of the time? If the latter, drop the clause?

This comment has been minimized.

Copy link
@benjyw

benjyw Apr 10, 2019

Author Contributor

We haven't deprecated fromfile yet, so it could in theory be set in an external plugin.

@ity

ity approved these changes Apr 8, 2019

@Eric-Arellano
Copy link
Contributor

left a comment

I'd love to see https://www.pantsbuild.org/options.html updated to describe how to read from a file. Feel free to do this in a followup if you want to avoid blocking this.

Thank you!

@benjyw benjyw merged commit 65ed665 into pantsbuild:master Apr 10, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@benjyw benjyw deleted the benjyw:fromfile_default_to_true branch Apr 10, 2019

@cosmicexplorer cosmicexplorer referenced this pull request Apr 15, 2019

Open

extract options parsing into its own library #7567

0 of 2 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.