Function to download history data

[llrs]

This is a draft after some iterations on the function (I originally used bash).
It depends on the number of columns on the file to simplify and merge the different files.

I hope it is clear enough (but maybe it could be simplified with a helper function).
It also iterates two times through the list of .csv files, that could be reduced but increasing the memory required earlier. (I don't think this is an issue and it could help if column names are reordered).
It doesn't check if data was recently downloaded or cache the data.

Let me know how do you see it and I'll finish writing the documentation.

[Bisaloo]

Before I go into the actual review, I have a couple of questions for you:

• Do you think we should reformat older files to make sure all files use the same format?
• Should we remove the non-csv files from the history branch? I believe cran-diagram.png was added by mistake and Hadley's analysis.R could be promoted to the main branch alongside your function to make it more discoverable.

[llrs]

Good questions.

• I don't like to tamper with original files. I would leave them as they are. Later on in a couple of years maybe the function could be modified to load data from a certain date (or last X period) which would exclude these files.
• That might be useful but they aren't much of a problem. I don't think the package is the right place to share analysis code on an article or branch. But sharing a minimal insight from the data collected might be useful for users of the dashboard (I would report numbers but hide or fold the code used).

[Bisaloo]

Looks good to me.

A couple of points to update:

• I'd like more comments about the various formats and when they were in use. I'll submit commit suggestions for this later
• Could you rename the file to download_history.R please?
• Could you add a roxygen title?
• Could you add a roxygen comment with #' @importFrom utils download.file read.csv unzip?
• Once the two points above have been addressed, could you run devtools::document()?

I'm happy to post commit suggestions or do the changes myself if convenient. Please let me know ☺️

[llrs]

Sorry, I almost forgot about this.
I'll do the changes and update the documentation (probably this week). But if you want to provide suggestion on the comments/dates go ahead.

[llrs]

I think I addressed all the point raised. I took the liberty to use desc::desc_normalize to arrange the order of dependencies after adding utils.

[llrs]

Gentle(?) reminder @Bisaloo and @maelle (I just remember about this and wasn't sure if I had updated the PR or not)

[Bisaloo]

Oh, I missed the fact you added comments with the dates so I wrote my own. I tried to merge what we both did. Please let me know what you think.

One final comment: I just realized now you're not in the authors of this package. Could you add yourself as aut please?

[llrs]

👍 I fixed the comments, reordered the code and I added myself as an author

[Bisaloo]

Thanks!