Skip to content

Latest commit

 

History

History
94 lines (86 loc) · 4.77 KB

File metadata and controls

94 lines (86 loc) · 4.77 KB

Code Review Guidelines

R Package Standards

  • This is a standard R package - all changes must maintain CRAN-compatible structure
  • NAMESPACE is auto-generated by roxygen2 - never edit it directly
  • DESCRIPTION must be kept valid with proper versioning
  • All exported functions require complete roxygen2 documentation

Code Style Requirements

  • Naming conventions: Use dotted.case, camelCase, or PascalCase consistently
    • Parameters/variables: dotted.case (e.g., response.format, text.variables, api.key)
    • Functions: camelCase or PascalCase or .camelCase (e.g., checkInputs, MakeOutput, or .nestedHelper)
      • Use PascalCase for exported functions (e.g., AddFormulaBars, RegressionTable)
      • Use camelCase for internal/helper functions (e.g., checkInputs)
      • Use .camelCase for internal nested functions e.g., within an internal function
      addOneToAll <- function(x) {
          .helpAdd <- function(y) {
              y + 1
          }
          vapply(x, .helpAdd, numeric(1L)
      }
      
  • Object name length: Maximum 50 characters
  • Assignment: Always use <- for assignment, never =
  • Pipes: Use native pipe |> for chaining operations
  • NULL coalescing: Use %||% operator for NULL defaults
  • Indentation: 4 spaces
  • Line length: Maximum 120 characters
  • No trailing whitespace
  • Consistent quotes: Always use double quotes " for strings. It is ok for quotes within strings to be single quotes ' but the parent string must use double quotes.
  • Prefer full words over abbreviations: e.g., response.format over resp.fmt
  • Function names: Use descriptive names that clearly indicate purpose: e.g. createSummaryTableForRegressionResults() rather than make()
  • Braces: Always use braces {} for functions, if, else if and else blocks. Except for the case of a single line statements. In particular, use Kernighan & Ritchie style braces.
    • e.g. The following is acceptable:
    y <- if (x > 0) x else -x
    w <- if (condition) {
        z <- computeSomething(x)
        foo(z)
    } else {
        z <- computeSomethingElse(x)
        bar(z)
    }
    
  • Redundant return: Avoid using return() unless exiting early from a function. The last evaluated expression is returned by default.

Function Design Patterns

  • Error handling: Use flipU::StopForUserError() for user-facing errors with clear, actionable messages
  • Return values: Functions used for validation/side effects should explicitly return NULL
  • Internal functions: Internal or helper functions should not be exported; use @noRd in roxygen comments
  • File organization: Group related functions into logical files (e.g., check.inputs.R, tidy.inputs.R)
  • Importing functions: Use @importFrom in roxygen comments for all external package functions used and avoid usage of :::.
  • Type checking: Don't use sapply. Use vapply with a specified return type for type safety.
  • Formatting numbers for printing: Use flipFormat::FormatAsReal() and flipFormat::FormatAsPercent() instead of base R formatting functions
  • Calculations: Use the following functions where possible:
    • verbs::Average instead of mean
    • verbs::Sum instead of sum
    • verbs::Min instead of min
    • verbs::Max instead of max
    • verbs::Variance instead of var
    • verbs::StandardDeviation instead of sd

Documentation Requirements

  • All exported functions must have complete roxygen2 documentation
  • Required tags: @param (for each parameter), @return, @export
  • Include @importFrom for all external package functions used
  • Optional but recommended: @details, @examples

Testing Requirements

  • Use testthat v3 framework (edition 3)
  • Test files must be in tests/testthat/ with test- prefix
  • Include snapshot tests where appropriate (_snaps/ directory)

Security & Best Practices

  • Never log or expose API keys or secrets

PR Review Focus Areas

  1. Breaking changes: Flag any changes to exported function signatures
  2. Documentation: Ensure all new/modified exports have complete roxygen2 docs
  3. Testing: Verify new functionality has corresponding tests
  4. Style consistency: Check adherence to naming convetions, <-, |> patterns
  5. Error messages: Ensure user-facing errors are clear and helpful
  6. Line length: Flag lines exceeding 120 characters
  7. Object names: Ensure names don't exceed 50 characters

Common Issues to Flag

  • Using = instead of <- for assignment
  • Missing or incomplete roxygen2 documentation
  • Direct API key exposure or logging
  • Inconsistent naming conventions (mixing dotted.case/camelCase inappropriately)
  • New dependencies without justification
  • Error messages that don't guide users to solutions