Closed Priceless-P closed 2 years ago
Hi @Priceless-P ,
Thanks for the PR.
Note sure what the Dog10K_Boxer_Tasha-seed
file has to do with this PR. Did you forget to resync your fork? Or maybe somehow you did something unintended with your various branches. Please remove.
Some nitpick comments about cosmetic:
Note that your if (!isTRUEorFALSE(replace))
statement is not indented properly.
No need to introduce 2 empty lines at lines 919-920. One is enough.
We use 4-space indentation increments for R code in Bioconductor. The code inside your if (file.exists(pkgdir)) { .. }
statement is using 2-space increments only.
Please align the replace=replace, verbose=verbose)
part of the 2 calls to forgeBSgenomeDataPkg()
that you modified with the seqs_srcdir
above it. That is, replace
should be horizontally aligned with seqs_srcdir
above it.
The \item{replace}{
line in man/BSgenomeForge.Rd
is not indented properly (this man page uses 2-space indentation increments so let's stick to that).
This line:
When set to TRUE \code{replace} replaces package directory if it already exists.
in the description of the replace
argument is more than 80-character long. I see this because I do all
my work in a 80-char wide terminal and the line gets wrapped up in my editor. We try to avoid lines that are
more than 80-character long (this is an old tradition with source code in general, not just in Bioconductor).
Please break the line.
I would add "the" between "replaces" and "package directory" in "When set to TRUE, \code{replace} replaces package directory if it already exists." Also a comma after "When set to TRUE".
Otherwise, the PR looks look. Good job!
All done. Please confirm if I did it right.
Note sure what the Dog10K_Boxer_Tasha-seed file has to do with this PR. Did you forget to resync your fork? Or maybe somehow you did something unintended with your various branches. Please remove.
I think it was the branches 😥. I have managed to remove it. Though it seems I lost myfix error
commit in the process.
Much cleaner. One last thing: the if else
statement inside your if (file.exists(pkgdir))
statement is still not indented properly. Pay attention to the } else {
line and to the line with the closing curly bracket.
Done 😊. Okay now?
Love it now!
I added a replace parameter to the
forgeBSgenomeDataPkg()
with a default value ofFALSE