Skip to content

Add optional output_dir to write_search_results_to_file()#11

Draft
jason-raitz wants to merge 1 commit intomainfrom
add-output-dir-to-write-method
Draft

Add optional output_dir to write_search_results_to_file()#11
jason-raitz wants to merge 1 commit intomainfrom
add-output-dir-to-write-method

Conversation

@jason-raitz
Copy link
Copy Markdown
Contributor

non-breaking change

- adds an optional output_dir parameter to overide the default
Copy link
Copy Markdown
Member

@anarchivist anarchivist left a comment

Choose a reason for hiding this comment

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

don't love the change (i was mostly fine with #10), but it's fine if we're refactoring it soon.

Comment thread tind_client/client.py

def write_search_results_to_file(
self, query: str = "", output_file_name: str = "tind.xml"
self, query: str = "", output_file_name: str = "tind.xml", output_dir: str = ""
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i understand this (based on getting up to speed on in slack) but i think adding output_dir honestly makes this more confusing. imo, output_file_name should just take a str | os.PathLike-typed value. but shrug if we're thinking about refactoring this soon anyway...

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.

2 participants