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

initial draft of fetch_qsf() #302

Closed
wants to merge 1 commit into from
Closed

initial draft of fetch_qsf() #302

wants to merge 1 commit into from

Conversation

jmobrien
Copy link
Collaborator

@jmobrien jmobrien commented Dec 7, 2022

Example of QSF downloading function, for review.

@jmobrien
Copy link
Collaborator Author

@juliasilge new function is for now in same file as its sibling functionfetch_description(). Love to hear any further thoughts thoughts about whether it would be better for users to have them together or separate.

@juliasilge
Copy link
Collaborator

This looks good to me so far! I think I'd probably keep it in the same file as the other, similar function.

@jmobrien
Copy link
Collaborator Author

Thoughts on folding it into fetch_description() fully? I lean towards feeling like this works differently enough to warrant being its own function, but I might just not be dialed in to the best arguments for keeping things more compact.

@juliasilge
Copy link
Collaborator

juliasilge commented Dec 20, 2022

Yeah, I think another function. What do you think about it being a function that is mainly called for its side effect of writing to disk? Like maybe change the name to write_qsf() to evoke write_csv(), write_feather(), and all the functions like that? It could invisibly return the QSF content still, if folks do want to wrangle it in R.

@juliasilge
Copy link
Collaborator

I just moved this repo's default branch from master to main; you can use usethis::git_default_branch_rediscover() to update your local setup.

@jmobrien
Copy link
Collaborator Author

Closing this. Covered by #319, which uses the write_qsf() naming and a more thoughtful implementation.

@jmobrien jmobrien closed this Jul 13, 2023
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