Skip to content

Conversation

@timcadman
Copy link
Contributor

@timcadman timcadman commented Dec 9, 2025

Instructions & checklist for PR author

Description of changes

  • Refactored ds.colnames to remove checks
  • Added two helper functions in utils.r: .set_datasources and .check_df_name_provided
  • Replaced checks in ds.colnames with these helper functions

Refactor instructions

  • Removed exists and isDefined from clientside function and add appropriate checks on server-side function
  • Removed any client-side code checking whether an object has been successfully created
  • Reviewed code to determine if additional refactoring could reduce calls to server-side package
  • Replaced check relating to datashield connections object with .set_datasources() (defined in utils.r)
  • If relevant, replaced argument check that dataset name has been provided to .check_df_name_provided() (defined in utils.r)

Testing instructions

  • Writen server-side unit tests for unhappy flow
  • Run devtools::test(filter = "smk-|disc|arg") and check it passes
  • Run devtools::check(args = '--no-tests') and check it passes (we run tests separately to skip performance checks)
  • Run devtools::build() and check it builds without errors

Instructions & checklist for PR reviewers

  • Run devtools::test(filter = "smk-|disc|arg") and check it passes
  • Run devtools::check(args = '--no-tests') and check it passes (we run tests separately to skip performance checks)
  • Run devtools::build() and check it builds without errors

@timcadman timcadman changed the base branch from master to v7.0-dev December 9, 2025 12:42
@timcadman timcadman changed the title V7.0 dev colnames V7.0 dev ds.colnames Dec 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants