Skip to content

Conversation

@zachhannum
Copy link
Contributor

Addresses #319.

@kevinushey let me know if there are any other reviewers we should include since we both contributed to the source changes!

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request adds support for the resourceProfile parameter to the rstudioapi::launcherSubmitJob function. This enhancement allows users to specify a named resource profile for job submission that must be defined in the cluster configuration.

  • Added resourceProfile parameter to launcherSubmitJob function with proper documentation
  • Implemented argument validation mechanism to ensure backward compatibility
  • Updated documentation and roxygen version

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
R/launcher-functions.R Added resourceProfile parameter to function signature and documentation
man/launcherSubmitJob.Rd Updated documentation to include the new resourceProfile parameter
R/utils.R Added checkApiArguments function for validating API arguments against RStudio versions
R/code.R Integrated argument validation using the new checkApiArguments function
vignettes/r-session.Rmd Minor title change
DESCRIPTION Updated RoxygenNote version

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

#'
#' @param resourceProfile A resource profile name to be used for the job. The
#' resource profile must be defined in the cluster configuration.
#'
Copy link

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

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

The documentation has trailing whitespace after the period on line 302. Remove the extra spaces for consistency with other parameter documentation.

Suggested change
#'
#'

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@kevinushey kevinushey left a comment

Choose a reason for hiding this comment

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

LGTM!

@kevinushey
Copy link
Contributor

The checks are failing here because the definitions used here are ancient, I think -- let me see if I can update those.

Copy link
Member

@gtritchie gtritchie left a comment

Choose a reason for hiding this comment

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

LGTM!

@zachhannum
Copy link
Contributor Author

Do we need to fix the test-coverage check @kevinushey? Looks like it's waiting on a test runner that will never come...

@kevinushey
Copy link
Contributor

Let me see if I can figure out the best way to update there.

@kevinushey kevinushey merged commit fba4729 into main Oct 3, 2025
6 checks passed
@kevinushey kevinushey deleted the zh-launcher-submit-job-resource-profile branch October 3, 2025 01:13
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

Successfully merging this pull request may close these issues.

4 participants