General Issue: Establish Codeowners for admiral codebase

[bms63]

Background Information

Ensure stability of code by having dedicated admiral devs automatically tagged in PRs that change essential R code of admiral.

• Identify relevant devs to be potential code owners
• Establish protocol on being a codeowner for admiral and what that entails
• Establish addition protocols for changing hands of CO, lack of time to be a COs, etc
• Identify files that should have codeowner attached to it and who should be that owner

Definition of Done

Checklist completed and Codeownership established for admiral codebase

[bundfussr]

What do you want to achieve by establishing codeowner?

It seems to contradict the community approach. I.e., the code is developed and maintained by the community. I also wonder how it should work considering the high turnover in the admiral dev team.

[bms63]

That core code being modified is brought to our attention - basically anything in the R folder that is modified should have one of us tagged in it ( us being me, you, @thomas-neitmann, @zdz2101, @sadchla-codes )... maybe @millerg23 and @galachad want in on the game as well. Others are welcome as well.

I was hoping the group of codeowners would stick to function themes that they are familiar with to help balance the load. I don't feel like it takes away the community feel - just lets us keep an eye on changes that folks propose. @cicdguy does this on admiralci. Basically, auto-assigns us as reviewers if our codeowner code is touched.

Helps also to let new and old developers find reviewers as we are already familiar with the code.

[bms63]

Happy medium - Try out with Lab and PK specific code. For example, lab excel file for grading.

@zdz2101 can you come up with list of potential lab and pk specific files

[zdz2101]

Lab based:

• ./vignettes/lab_grading.Rmd
• ./inst/adlb_grading/
• ./R/derive_var_atoxgr.R
• ./data/atoxgr_criteria_ctcv5.rda

PK based

• ./inst/templates/ad_adppk.R
• ./R/compute_kidney.R
• ./vignettes/pk_adnca.Rmd

Are you both (@millerg23 for labs, @jeffreyad for pk) okay with being added as codeowners for the corresponding files? In short you will always be and automatically pinged/notified if there is a PR that includes the respective files

[jeffreyad]

@zdz2101 I'm fine with being added to help test this functionality. Please also add ./inst/templates/ad_adpc.R

[millerg23]

yes @zdz2101 thats fine by me. If you need to assign someone to templates, then I can cover the ADLB template (and ADVS/ADEG).

[zdz2101]

@pharmaverse/admiral do we want to re-visit this idea of codeowners

[bms63]

While I think this is a cool idea and was initially enthused about it - I now feel like it is just more bureaucracy for us. The risk of someone doing something untoward to our files is pretty minimal with all our infrastructure.

I think the most important takeaway is identifying people in our weekly meetings to do code review as this was one of the code owners original purposes. I feel like if we stay on top of PRs and ask for reviewers the. We should be okay.

Any thought @pharmaverse/admiral ?

[cicdguy]

I use CODEOWNERS as a way to notify the team that a new contribution has been made, especially if a certain team member is unavailable to review the code (vacation etc).

[millerg23]

@bms63 you are very good at keeping on top of the PRs, and like you say, we can assign reviewers at the regular stand-ups. If you want to try it out with just labs and PK, I am fine with that also. I am not sure how much extra code is required to implement this, and how easy it it to maintain. I am all for less code to maintain in general, unless its really needed.

[bms63]

I think the code is minimal to implement. It is just the documentation that is putting me off from wanting to implement it.

If @zdz2101 or someone else wants to make a brief mention of it in the PR docs in admiraldev then I am okay with proceeding.

[bms63]

I will update the docs on admiraldev and put in some language around codeownership expectations.