search

pharmaverse / admiral.test

Test SDTM data for use with admiral
Other
2 stars 1 forks source link

closes #51 - Add new variables to MH #56

Closed yangc71 closed 2 years ago

yangc71 commented 2 years ago

Close #51

yangc71 commented 2 years ago

Hi @rossfarrugia, @bms63 I added some variables to MH based on dummy data, could you please take a look and let me know if any changes are required? Also, after running devtools::documents() the documentation for ‘admiral_ds’ ‘admiral_mh’ ‘admiralsuppds’ are deleted (because they are renamed to raw*). I'll add those back to data.R tomorrow. Thanks!

rossfarrugia commented 2 years ago

thanks @yangc71 for the efforts! will leave this to @bms63 to review as I'm not working on ADMH.

bms63 commented 2 years ago

@yangc71 Can you fix these merge conflicts? Need to merge in devel to get the latest checks.

yangc71 commented 2 years ago

@yangc71 Can you fix these merge conflicts? Need to merge in devel to get the latest checks.

Fixed, thanks!

bms63 commented 2 years ago

Also please put yourself as author in DESCRIPTION file and for admiral_mh like Gopi and Madhi did for ds and pc. Another R package with Annie Yang as an author!!

yangc71 commented 2 years ago

Can you also run styler on the code?

Sure, I ran styler in the latest commit. I wonder if we should put this in the PR checklist too, to run styler?

bms63 commented 2 years ago

Can you also run styler on the code?

Sure, I ran styler in the latest commit. I wonder if we should put this in the PR checklist too, to run styler?

Yes! Do you mind updating the template?

yangc71 commented 2 years ago

Thanks, I updated the PR template. Please let me know if further changes are needed.