search

morinlab / GAMBLR.results

A collection of functions to access results of the Genomic Analysis of Mature B-cell Lymphomas
MIT License
0 stars 0 forks source link

Harmonize naming of the parameters in get_sample_cn_segments issue #4 #15

Closed mattssca closed 9 months ago

mattssca commented 9 months ago

This PR resolves issue #4.

vladimirsouza commented 9 months ago

This is a great fix!

vladimirsouza commented 9 months ago

I see there is no need for @import GAMBLR.helpers or prefixing GAMBLR.helpers::check_config_value, because in NAMESPACE there is importFrom(GAMBLR.helpers,check_config_value). But aren't we prefixing functions from other child repos for tracking purposes?

vladimirsouza commented 9 months ago

Sorry. I was typing a comment but closed the PR by accident (getting used to a new [and nice] keyboard).

mattssca commented 9 months ago

Seems like an awesome keyboard lol.

Right, I actually overlooked the package prefixes for check_config_value. Are you saying it's ok since this function is being imported by NAMESPACE? Or do you want me to add the package prefixes to the internal call of check_config_value?

mattssca commented 9 months ago

I'll add the prefixes in a separate commit so it's consistent throughout the code base.

vladimirsouza commented 9 months ago

The function is working perfectly as it is. I just wanted to ask to confirm that.

mattssca commented 9 months ago

I've added the prefixes now, for consistency. Thanks for catching this.