refactor target package

[harp-intel]

No description provided.

[Copilot]

Pull Request Overview

This PR refactors the target package to improve the separation of target functionalities and enhance naming consistency across methods. Key changes include the implementation of local target methods, extraction of helper functions into a separate file, and updates in common and CLI logic to reflect the new target interface.

Reviewed Changes

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

File	Description
internal/target/local_target.go	Introduces local target methods with support for sudo, reports, and filesystem operations.
internal/target/helpers.go	Adds helper functions to run local commands and extract system information.
internal/common/common.go	Updates target reference usage in report generation and output handling.
cmd/lock/lock.go	Modifies pullDataFiles to support progress updates during file retrieval.

Comments suppressed due to low confidence (2)

internal/target/local_target.go:60

• The return variable 'family' is misleading since the method returns t.model. Consider renaming it to 'model' to reflect its purpose.

func (t *LocalTarget) GetModel() (family string, err error) {

internal/target/local_target.go:74

• The return variable 'arch' is misleading since the method returns t.vendor. Consider renaming it to 'vendor' to accurately represent the value being returned.

func (t *LocalTarget) GetVendor() (arch string, err error) {

[Copilot]

Pull Request Overview

This PR refactors the target package to improve command execution handling, privilege elevation, and to streamline target data management. Key changes include:

• Implementation of new functions for local target management (e.g., RunCommand, file operations) in internal/target/local_target.go.
• Updates to helper functions in internal/target/helpers.go for asynchronous commands and improved error handling.
• Refactoring of TargetScriptOutputs in internal/common/common.go to replace the target object with a targetName field, and corresponding updates in report and script output handling.
• Modifications to the lock command to incorporate progress updates when pulling data files.

Reviewed Changes

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

File	Description
internal/target/local_target.go	New local target functions and file operations
internal/target/helpers.go	Asynchronous command execution and error handling improvements
internal/common/common.go	Refactored TargetScriptOutputs to use targetName instead of target
cmd/lock/lock.go	Enhanced pullDataFiles with progress status updates

Comments suppressed due to low confidence (1)

internal/target/local_target.go:159

• The PullFile function currently delegates to PushFile, which may not match the intended behavior for pulling files. Consider implementing a dedicated file retrieval mechanism if different semantics are required.

return t.PushFile(srcPath, dstPath)

[Copilot]

Pull Request Overview

This PR refactors the target package and reporting components to improve code clarity and consistency. Key changes include:

• Adding new methods to LocalTarget for command execution, file handling, and system info retrieval.
• Updating reporting types to use a target name instead of a full target object.
• Enhancing progress reporting in the lock command.

Reviewed Changes

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

File	Description
internal/target/local_target.go	Added LocalTarget methods and refined sudo/command execution logic.
internal/target/helpers.go	Introduced helper functions for command execution with timeout and input.
internal/common/common.go	Updated TargetScriptOutputs structure and report generation to use targetName.
cmd/lock/lock.go	Modified pullDataFiles to include progress status updates and error handling.

Comments suppressed due to low confidence (1)

internal/target/local_target.go:25

• The parameter 'argNotUsed' is never used. Consider removing it (or renaming it to indicate its purpose) if it is not required to improve clarity.

func (t *LocalTarget) RunCommand(cmd *exec.Cmd, timeout int, argNotUsed bool) (stdout string, stderr string, exitCode int, err error) {

[Copilot]

Pull Request Overview

A refactor of the target package that modernizes command execution and target data handling while aligning reporting with a new target name field. Key changes include a reorganization of target methods in internal/target/local_target.go; updates to LKM helper functions in internal/target/helpers.go; refactoring of TargetScriptOutputs to use a targetName rather than a target reference in internal/common/common.go; and modifications in cmd/lock/lock.go to include progress status updates during file pulling.

Reviewed Changes

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

File	Description
internal/target/local_target.go	Implements various target operations and command execution functions with minor refactors.
internal/target/helpers.go	Provides helper functions for LKM management and local command execution with timeouts.
internal/common/common.go	Updates TargetScriptOutputs to use targetName and revises AdhocFunc signature for progress.
cmd/lock/lock.go	Updates pullDataFiles to include a progress update and improves error/status handling.

Comments suppressed due to low confidence (2)

internal/target/local_target.go:25

• [nitpick] The parameter 'argNotUsed' is not utilized within the function; consider removing it or renaming it to clarify its purpose.

func (t *LocalTarget) RunCommand(cmd *exec.Cmd, timeout int, argNotUsed bool) (stdout string, stderr string, exitCode int, err error) {

internal/target/local_target.go:159

• The PullFile function currently calls PushFile, which may be counterintuitive for a pull operation; please confirm if this behavior is intended or if it should implement its own logic for pulling files.

return t.PushFile(srcPath, dstDir)

[Copilot]

Pull Request Overview

This PR refactors the target package and associated components to improve how target operations and reporting are handled.

• Updated functions in the target package to separate sudo handling, command execution, and temporary directory management.
• Refactored reporting and script helper functions to use a simplified target identifier and improved error/status handling.
• Modified the lock command implementation to include status updates during file retrieval.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
internal/target/local_target.go	Added methods for sudo handling and command execution; refactored GetUserPath.
internal/target/helpers.go	Introduced helper functions for LKM installation and command execution.
internal/script/script.go	Updated script parsing using new iteration pattern with strings.SplitSeq.
internal/report/table_helpers.go	Updated table helper functions to iterate over string splits correctly.
internal/report/table_defs.go	Refactored report generation to use a simplified target name field.
internal/common/common.go	Modified TargetScriptOutputs to use targetName instead of a target object.
cmd/lock/lock.go	Extended pullDataFiles to issue proper status updates and error handling.

[Copilot]

The loop is currently iterating over indices rather than the path values. Change to 'for _, p := range strings.SplitSeq(pathEnv, ":")' so that 'p' represents the actual path string for use with filepath.Glob.

Suggested change

	for p := range strings.SplitSeq(pathEnv, ":") {
	for _, p := range strings.SplitSeq(pathEnv, ":") {

[Copilot]

The parameter 'argNotUsed' is never used in the function body. Consider removing it to simplify the API.

Suggested change

	func (t *LocalTarget) RunCommand(cmd *exec.Cmd, timeout int, argNotUsed bool) (stdout string, stderr string, exitCode int, err error) {
	func (t *LocalTarget) RunCommand(cmd *exec.Cmd, timeout int) (stdout string, stderr string, exitCode int, err error) {