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

Get R check passing #9

Merged
merged 5 commits into from
May 1, 2023
Merged

Get R check passing #9

merged 5 commits into from
May 1, 2023

Conversation

ablack3
Copy link
Contributor

@ablack3 ablack3 commented Apr 27, 2023

Actually I think I still have a few things to do still.

  • Please add \value to .Rd files regarding exported methods and explain
    the functions results in the documentation. Please write about the
    structure of the output (class) and also what the output means. (If a
    function does not return a value, please document that too, e.g.
    \value{No return value, called for side effects} or similar)
    Missing Rd-tags in up to 39 .Rd files, e.g.:
    age.Rd: \value
    atLeast.Rd: \value
    atMost.Rd: \value
    attrition.Rd: \value
    censoringEvents.Rd: \value
    ...
    All functions need @return tags.

  • \dontrun{} should only be used if the example really cannot be executed
    (e.g. because of missing additional software, missing API keys, ...) by
    the user. That's why wrapping examples in \dontrun{} adds the comment
    ("# Not run:") as a warning for the user. Does not seem always
    necessary. Please replace \dontrun with \donttest.

  • Please unwrap the examples if they are executable in < 5 sec, or replace
    dontrun{} with \donttest{}.

@ablack3 ablack3 requested a review from mdlavallee92 April 27, 2023 20:31
@codecov-commenter
Copy link

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.21 🎉

Comparison is base (486605b) 72.80% compared to head (c31d3e3) 73.02%.

❗ Current head c31d3e3 differs from pull request most recent head 999e692. Consider uploading reports for the commit 999e692 to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##             main       #9      +/-   ##
==========================================
+ Coverage   72.80%   73.02%   +0.21%     
==========================================
  Files          11       11              
  Lines         923      923              
==========================================
+ Hits          672      674       +2     
+ Misses        251      249       -2     
Impacted Files Coverage Δ
R/attributes-concept.R 65.90% <ø> (ø)
R/attributes-nested.R 30.00% <ø> (ø)
R/collectCodesetId.R 87.96% <ø> (ø)
R/criteria.R 62.65% <ø> (ø)
R/exit.R 84.44% <ø> (ø)
R/query.R 63.15% <ø> (ø)
R/cohort.R 77.57% <100.00%> (+2.09%) ⬆️
R/conceptSet.R 70.00% <100.00%> (-0.12%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@mdlavallee92 mdlavallee92 left a comment

Choose a reason for hiding this comment

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

This looks fine. Going to pull this into the main OdyOSG.

In the future this should be the process either:

  • Commit to OdyOSG/Capr main -> PR OdyOSG main to ohdsi/Capr develop
  • Branch on ohdsi/Capr PR to develop

I will take care of all the package maintenance with the site and what not when the PR is going to ohdsi/Capr main. Dont do it downstream because it ends up having to be redone anyways.

Thanks for your help! Let me know how much more effort is needed for the documentation examples

@mdlavallee92 mdlavallee92 merged commit ad0e40c into main May 1, 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.

3 participants