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

refactor ELT + analysis codebase #52

Closed
5 tasks
maxheld83 opened this issue Dec 10, 2020 · 1 comment
Closed
5 tasks

refactor ELT + analysis codebase #52

maxheld83 opened this issue Dec 10, 2020 · 1 comment
Milestone

Comments

@maxheld83
Copy link
Contributor

maxheld83 commented Dec 10, 2020

this is a bit of a big issue, but just to keep track of what should be factored out:

  • the rmarkdown template (for the mail) strictly should not do any work, but should merely define the presentation of results (i.e. the order of tables and perhaps some commentary).
    • Among other things, the rmarkdown template currently creates the excel output, which makes this quite brittle (and required the hack-fix for send attachment #44
    • the substantive documentation in the template (i.e. what this or that means, not just "hello"-boilerplate) should be in the roxgen2 documentation for the respective functions.
      We can then figure out a way to dynamically pull this into the rmarkdown template.
      (I know that it's easily possible to pull in random rmarkdown chunks into the roxygen docs, but it would be even more expressive to do it the other way around -- will have to investigate).
      The logic here is that this kind of info really belongs to the functions, where it can be maintained together, tested, etc.
    • no dependencies in rmarkdown templates, in particular, or at least not without programmatically rendereing the template as part of the check.
      If there is a missing dependency as in https://github.com/subugoe/metacheck/actions/runs/413556533 this creates a pretty thorny failure mode, because the missing dep isn't captured by R CMD check.
  • simply no r code that isn't a function and isn't covered by at least R CMD check and friends, ideally test() as well.
    tbc.
@maxheld83 maxheld83 added this to the Early Release milestone Dec 10, 2020
maxheld83 added a commit that referenced this issue Dec 10, 2020
This is a pretty bad hackfix that is untested and may easily break.
A proper solution will require refactoring as per #44.
maxheld83 added a commit that referenced this issue Dec 10, 2020
This is a pretty bad hackfix that is untested and may easily break.
A proper solution will require refactoring as per #44.
@maxheld83
Copy link
Contributor Author

this now lives in #69

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

No branches or pull requests

1 participant