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

130 DST01 sample data review #131

Merged
merged 3 commits into from
Mar 11, 2022
Merged

Conversation

BFalquet
Copy link
Contributor

@BFalquet BFalquet commented Feb 4, 2022

close insightsengineering/chevron-tasks#43

implement suggested changes (except the null digit problem which is an rtables problem)
thank you for the review

@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 2022

Code Coverage Summary

Filename                           Stmts    Miss  Cover    Missing
-------------------------------  -------  ------  -------  ---------
R/aet01.R                            142     142  0.00%    42-335
R/aet02.R                            163     163  0.00%    55-418
R/aet03.R                             61      61  0.00%    44-145
R/aet04.R                             67      67  0.00%    53-159
R/assertions.R                        23      23  0.00%    22-47
R/assertthat.R                         1       1  0.00%    3
R/cmt01a.R                           132     132  0.00%    52-376
R/cmt02_pt.R                          29      29  0.00%    36-92
R/dm_explicit_na.R                    44      44  0.00%    48-129
R/dmt01.R                             20      20  0.00%    48-113
R/dst01.R                            212     212  0.00%    5-551
R/egt01.R                             39      39  0.00%    45-122
R/ext01.R                             41      41  0.00%    40-176
R/gen_args.R                           1       1  0.00%    18
R/lbt01.R                             34      34  0.00%    43-111
R/mht01.R                             52      52  0.00%    41-125
R/sample_study_object.R               14      14  0.00%    14-35
R/standard_data_preprocessing.R      170     170  0.00%    38-456
R/utils.R                            130     130  0.00%    17-374
R/vst01.R                             40      40  0.00%    46-125
TOTAL                               1415    1415  0.00%

Results for commit: 7d8b91300db3123ecd75f39609aa7c5183b6f2e4

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@BFalquet BFalquet requested a review from barnett11 February 9, 2022 10:08
Copy link
Contributor

@barnett11 barnett11 left a comment

Choose a reason for hiding this comment

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

Further feedback added to #130

@wangh107 wangh107 self-requested a review March 10, 2022 21:06
Copy link
Contributor

@wangh107 wangh107 left a comment

Choose a reason for hiding this comment

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

approve this PR with comment:
not sure whether 4 (1%) should be 4 (1.0%). If that's the case, then rtables needs to solve it in future.
Screen Shot 2022-03-10 at 1 06 12 PM

@barnett11 barnett11 self-requested a review March 11, 2022 12:14
Copy link
Contributor

@barnett11 barnett11 left a comment

Choose a reason for hiding this comment

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

All changes look good thanks @BFalquet
Two outstanding issues on alignment and 0% can be looked at later/resolved in rtables hopefully

@BFalquet BFalquet merged commit 60c318f into main Mar 11, 2022
@BFalquet BFalquet deleted the 130_dst01-sample-data-review@main branch March 11, 2022 14:47
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