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

Roxygen documentation rewrite #198

Merged
merged 3 commits into from
Mar 13, 2019
Merged

Conversation

jayqi
Copy link
Collaborator

@jayqi jayqi commented Mar 3, 2019

Overhaul of our Roxygen documentation to resolve #192.

Changes to Reporter Documentation

  • New articles PackageReporters and NetworkReporters contain sections defining their respective public interfaces.
    • These are hidden from documentation indices (in RStudio or pkgdown) by using @keywords internal. They are still findable if you do ?PackageReporters, so there is still a meaningful description written for people who bother looking for them.
    • Sections for various public interfaces are defined using rsixygen-generated doc skeletons.
    • The articles for all user-facing reporters inherit these sections. This way, each one is standalone (contains full documentation for public interface), but underlying roxygen content is not duplicated for ease of maintenance.
  • User-facing reporter articles document NULL objects rather than the class constructors themselves. This way, we can leave out the completely useless @Usage and @Format sections.
  • Standardizing around the general language of "network reporters" in the documentation. I don't think we really had any term here before, and I've personally used "graph reporters" when it came up. Since this is user facing and refers to networks in a package, I think the word "network" is the right thing here. (See discussion in Major Refactor to GraphReporters #181.) (Also relevant to Update Style Guide in CONTRIBUTING.md with Function Naming Conventions #183)

Summary of Other Changes:

  • AbstractPackageReporter and AbstractGraphReporter are no longer exported. Resolves Exporting of Abstract Classes #190.
  • Removed docs_shared. This was only getting used in one place (CreatePackageReport) so I just copied them in and got rid of it.
  • Created package documentation in pkgnet.R. This will show up with you do ?pkgnet or ?pkgnet-package. Resolves Write roxygen article for package #193.

@codecov-io
Copy link

codecov-io commented Mar 3, 2019

Codecov Report

Merging #198 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #198   +/-   ##
=======================================
  Coverage   91.52%   91.52%           
=======================================
  Files          11       11           
  Lines         920      920           
=======================================
  Hits          842      842           
  Misses         78       78
Impacted Files Coverage Δ
R/PackageDependencyReporter.R 81.65% <ø> (ø) ⬆️
R/CreatePackageReport.R 98.24% <ø> (ø) ⬆️
R/PackageFunctionReporter.R 93.33% <ø> (ø) ⬆️
R/DefaultReporters.R 100% <ø> (ø) ⬆️
R/PackageInheritanceReporter.R 100% <ø> (ø) ⬆️
R/GraphClasses.R 93.82% <ø> (ø) ⬆️
R/AbstractGraphReporter.R 86.28% <ø> (ø) ⬆️
R/AbstractPackageReporter.R 100% <ø> (ø) ⬆️
R/PackageSummaryReporter.R 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f98b368...1c1c13f. Read the comment docs.

R/pkgnet.R Show resolved Hide resolved
Copy link
Collaborator

@bburns632 bburns632 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one function to add back to the roxygen docs, so it gets NAMESPACE'd: knitr::opts_chunk. Also, can you comment on the dual families for functions.

R/PackageInheritanceReporter.R Show resolved Hide resolved
R/PackageFunctionReporter.R Show resolved Hide resolved
R/CreatePackageReport.R Show resolved Hide resolved
R/PackageDependencyReporter.R Show resolved Hide resolved
@bburns632 bburns632 merged commit 71d7fa8 into uptake:master Mar 13, 2019
@jayqi jayqi added this to the Release v0.4.0 milestone Mar 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants