Skip to content

Conversation

@harp-intel
Copy link
Contributor

This pull request updates the way scripts are run on targets throughout the codebase, primarily by refactoring the script.RunScripts function to accept additional parameters for status updates and status messages. This change improves progress reporting and status tracking during script execution, and standardizes how status updates are handled across multiple modules.

Refactoring for improved status reporting:

  • Updated the script.RunScripts function signature in internal/script/script.go to accept statusUpdate and collectingStatus parameters, allowing callers to provide status update callbacks and custom status messages.
  • Added status update calls before preparing the target and before running scripts in script.RunScripts, enhancing progress visibility for users. [1] [2]

Codebase-wide adoption of new interface:

  • Refactored all calls to script.RunScripts in cmd/config/set.go, cmd/config/config.go, cmd/metrics/metadata.go, and internal/common/common.go to use the new parameters, passing appropriate status update functions and messages or nil/empty string where not needed. [1] [2] [3] [4] [5] [6] [7] [8] [9]

Dependency update:

  • Added an import for perfspect/internal/progress in internal/script/script.go to support the new status update functionality.

Signed-off-by: Harper, Jason M <jason.m.harper@intel.com>
@harp-intel harp-intel requested a review from Copilot November 30, 2025 01:31
Copy link
Contributor

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 PR enhances progress reporting during script execution by refactoring the script.RunScripts function to accept status update callbacks and messages as parameters. This allows callers to provide custom status messages and enables the display of a "preparing to collect data" message before target preparation, improving visibility into script execution progress.

Key changes:

  • Modified script.RunScripts signature to accept statusUpdate and collectingStatus parameters
  • Added status update calls in script.RunScripts before preparing targets and before running scripts
  • Updated all callers across the codebase to pass the new parameters

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
internal/script/script.go Added status update parameters to RunScripts function and inserted status update calls before target preparation and script execution
internal/common/common.go Refactored to pass status update function and message to RunScripts instead of calling status update before the function
cmd/metrics/metadata.go Updated RunScripts calls to include nil and empty string for new parameters where status updates are not needed
cmd/config/set.go Updated multiple RunScripts calls to include nil and empty string for new parameters
cmd/config/config.go Refactored to pass status update function and message to RunScripts instead of calling status update before the function

@harp-intel harp-intel merged commit 66c5219 into main Nov 30, 2025
7 of 8 checks passed
@harp-intel harp-intel deleted the statusprep branch November 30, 2025 16:03
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.

2 participants