Skip to content

Martin/rea 132 create socket scan create reach option for running tier 1#704

Merged
jdalton merged 4 commits intomainfrom
martin/rea-132-create-socket-scan-create-reach-option-for-running-tier-1
Aug 11, 2025
Merged

Martin/rea 132 create socket scan create reach option for running tier 1#704
jdalton merged 4 commits intomainfrom
martin/rea-132-create-socket-scan-create-reach-option-for-running-tier-1

Conversation

@mtorp
Copy link
Contributor

@mtorp mtorp commented Aug 11, 2025

No description provided.

@mtorp mtorp requested review from barslev and jdalton August 11, 2025 08:08
Copy link
Contributor

@pvdz pvdz left a comment

Choose a reason for hiding this comment

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

Please have a look at how the other commands do the CResult flow; cmd_* -> handle_ -> output_
I'll leave this to @jdalton but leaving some comments to help you along :)

Comment on lines +201 to +206
packagePaths = packagePaths.filter(
p =>
/* Exclude DOT_SOCKET_DOT_FACTS_JSON from previous runs */ !p.includes(
constants.DOT_SOCKET_DOT_FACTS_JSON,
),
)
Copy link
Contributor

Choose a reason for hiding this comment

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

The parameter packagePaths is being reassigned without proper declaration. According to the Google TypeScript Style Guide, variables requiring reassignment should use let instead of const. Either declare the parameter as let packagePaths in the function signature, or create a new variable with let for the filtered result to avoid reassigning the parameter directly.

Suggested change
packagePaths = packagePaths.filter(
p =>
/* Exclude DOT_SOCKET_DOT_FACTS_JSON from previous runs */ !p.includes(
constants.DOT_SOCKET_DOT_FACTS_JSON,
),
)
const filteredPackagePaths = packagePaths.filter(
p =>
/* Exclude DOT_SOCKET_DOT_FACTS_JSON from previous runs */ !p.includes(
constants.DOT_SOCKET_DOT_FACTS_JSON,
),
)

Spotted by Diamond (based on custom rule: Custom rules)

Is this helpful? React 👍 or 👎 to let us know.

Copy link
Contributor

@jdalton jdalton left a comment

Choose a reason for hiding this comment

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

Minor nits, AI suggestions, and comments. Other than that it looks good. Thanks @pvdz for review and @mtorp for the PR!

@jdalton jdalton merged commit 201a81d into main Aug 11, 2025
31 checks passed
@jdalton jdalton deleted the martin/rea-132-create-socket-scan-create-reach-option-for-running-tier-1 branch August 11, 2025 14:57
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.

3 participants