Comments from the Bioconductor reviewer

zhiiiyang commented 3 years ago

NanoStringNCTools review (https://github.com/Bioconductor/Contributions/issues/1815#issuecomment-750319807)

NAMESPACE

[x] being done manually, instead of by roxygen2. Please make sure functions/clases exported are all needed.

Build report warnings

NOTE: These are the NOTES i'm particular about, please address these. The rest of them are also solvable for the most part (at least the ones you see on the linux builder)

[x] NOTE: Avoid sapply(); use vapply()
[x] NOTE: Consider adding runnable examples to the following man pages which document exported objects: geom_beeswarm_interactive.Rd, SignatureSet.Rd
[x] NOTE: Consider adding a NEWS file, so your package news will be included in Bioconductor release announcements.
[x] The windows build machine issue can be addressed by @hpages better.

R

[x] There is a NOTE about indentation too(2 spaces vs 4 spaces), please

check http://bioconductor.org/developers/how-to/coding-style/ .

[x] Remove commented out lines of code.
[x] I'm not sure why your zzz.R has the 'testPackage' function called inside of it. This usually isn't the way zzz.R is used.

zhiiiyang commented 3 years ago

I remove zzz.R after discussing it with Dave and Rona.

NicoleEO commented 3 years ago

@dnadave for the top item, they just want us to double check that we have everything we need exported right? There's no edit to the code if we feel all is as it should be.

dnadave commented 3 years ago

I don't think we need to export any other functions than what we have in the NAMESPACE file now. I think we're good.

On Tue, Jan 5, 2021 at 3:50 PM Nicole Ortogero notifications@github.com wrote:

@dnadave https://github.com/dnadave for the top item, they just want us to double check that we have everything we need exported right? There's no edit to the code if we feel all is as it should be.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Nanostring-Biostats/NanoStringNCTools/issues/5#issuecomment-754971050, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJVFGZKDJCMAECQEJ4R52LSYOQVRANCNFSM4VMKX6UA .

-- David Henderson, Ph.D. 18476 47th Place NE Lake Forest Park, WA 98155 206-794-8552

NicoleEO commented 3 years ago

Left one sapply in esBy as this is sort of an extension of the sapply function for our class.

Nanostring-Biostats / NanoStringNCTools

Comments from the Bioconductor reviewer #5