subugoe / hoad

Deprecated: Please check https://github.com/subugoe/hoaddash
https://github.com/subugoe/hoaddash
GNU Affero General Public License v3.0
15 stars 4 forks source link

factor out license patterns #213

Closed maxheld83 closed 4 years ago

maxheld83 commented 4 years ago

part of path was missing & licence_patterns is needed as a vector

maxheld83 commented 4 years ago

csv file from https://github.com/subugoe/hoad/commit/3d8b8f4a69b022812ecbe59bad66df445e105c4d is still missing for this PR to work.

(Apologies @jhoeffler, this was my mistake, I think my reverts on #172 killed the csv, but I'll carry it over in the merge).

codecov-commenter commented 4 years ago

Codecov Report

Merging #213 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #213   +/-   ##
=======================================
  Coverage   62.22%   62.22%           
=======================================
  Files           4        4           
  Lines          45       45           
=======================================
  Hits           28       28           
  Misses         17       17           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 52b3ffe...8e3cc1e. Read the comment docs.

maxheld83 commented 4 years ago

Ok, I have now refactored @jhoeffler refactor 🙃 as a properly documented and tested R object inside the package. This, as I have been suggesting for some time, should, as far as possible, be the modus operandi going forward for all new additions/refactors/etc. Slowly but steadily, we should be factoring out ever more stuff from monolithic functions and scripts into such small, tested and documented building blocks.

This seems a bit weird in this case, because the object is just a tribble, and doesn't really do much. Though considering how much debate there was around this and how many comments were in the script including sources, etc, I think this is how we should be moving forward even for relatively minor components.

In this case, I:

This is just a dress rehearsal for the real deal in #47 and #81.

Unfortunately, the above diff is a bit messed up, because there were:

A short note on where to store such things: inst/extadata/*.csv have some disadvantages:

I'm going to leave this open, to wait for @jhoeffler's review, and will then merge it if it's allright.