Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Comments from the Bioconductor reviewer #5

Closed
8 tasks done
zhiiiyang opened this issue Dec 28, 2020 · 4 comments
Closed
8 tasks done

Comments from the Bioconductor reviewer #5

zhiiiyang opened this issue Dec 28, 2020 · 4 comments
Assignees

Comments

@zhiiiyang
Copy link
Contributor

zhiiiyang commented Dec 28, 2020

NanoStringNCTools review (Bioconductor/Contributions#1815 (comment))

NAMESPACE

  • 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)

  • NOTE: Avoid sapply(); use vapply()

  • NOTE: Consider adding runnable examples to the following man
    pages which document exported objects:
    geom_beeswarm_interactive.Rd, SignatureSet.Rd

  • NOTE: Consider adding a NEWS file, so your package news will be
    included in Bioconductor release announcements.

  • The windows build machine issue can be addressed by @hpages better.

R

  • There is a NOTE about indentation too(2 spaces vs 4 spaces), please

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

  • Remove commented out lines of code.

  • 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 pushed a commit that referenced this issue Jan 5, 2021
@zhiiiyang
Copy link
Contributor Author

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

@NicoleEO
Copy link
Collaborator

NicoleEO commented Jan 5, 2021

@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
Copy link
Contributor

dnadave commented Jan 6, 2021 via email

@NicoleEO
Copy link
Collaborator

NicoleEO commented Jan 6, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants