Skip to content

Conversation

@GordonSmith
Copy link
Member

@GordonSmith GordonSmith commented Jun 30, 2025

Fixes #4392

Checklist:

  • The commit message is properly formatted and free of typos.
    • The commit message title makes sense in a changelog, by itself.
    • The commit message includes a "fixes" reference if appropriate.
    • The commit is signed.
  • The change has been fully tested:
    • I have viewed all related gallery items
    • I have viewed all related dermatology items
  • There are no similar instances of the same problem that should be addressed
    • I have addressed them here
    • I have raised new issues to address them separately

Testing:

@GordonSmith GordonSmith requested review from Copilot and jeclrsg June 30, 2025 13:44
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

Adds a recursive fetch for nested logical files in the DFU service and exposes it via a new helper on the LogicalFile object.

  • Introduces recursiveFetchLogicalFiles in wsDFU.ts to gather subfiles and superfiles in a single call
  • Adds fetchAllLogicalFiles on LogicalFile to invoke the new recursive method
  • Does not include accompanying documentation or tests for the new recursive behavior

Reviewed Changes

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

File Description
packages/comms/src/services/wsDFU.ts New recursiveFetchLogicalFiles method for deep traversal of sub- & superfiles
packages/comms/src/ecl/logicalFile.ts Added fetchAllLogicalFiles wrapper that calls into the recursive service
Comments suppressed due to low confidence (3)

packages/comms/src/services/wsDFU.ts:18

  • Add a JSDoc comment above this method to explain its purpose, parameters, and return value for future maintainers.
    async recursiveFetchLogicalFiles(superFiles: { NodeGroup: string, Name: string }[]): Promise<string[]> {

packages/comms/src/services/wsDFU.ts:32

  • Add unit tests covering nested recursion, including scenarios with multiple levels of superfiles, to ensure correct termination and result flattening.
        return logicalFiles.concat(childSuperFiles.length ? await this.recursiveFetchLogicalFiles(childSuperFiles) : []);

packages/comms/src/ecl/logicalFile.ts:180

  • The recursiveFetchLogicalFiles method expects objects with NodeGroup and Name properties, but this may not satisfy that shape. Consider passing { NodeGroup: this.NodeGroup, Name: this.Name } or similar.
        return this.connection.recursiveFetchLogicalFiles([this]);

@GordonSmith
Copy link
Member Author

@jeclrsg can you review

@GordonSmith GordonSmith merged commit 8941789 into hpcc-systems:candidate-2.x.x Jul 2, 2025
5 checks passed
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