Skip to content

Switch to folders.FindDirWithLeaf - #1963

Merged
denik merged 7 commits into
mainfrom
DECO-24125--cleanup-vfs-leaf
Dec 11, 2024
Merged

Switch to folders.FindDirWithLeaf#1963
denik merged 7 commits into
mainfrom
DECO-24125--cleanup-vfs-leaf

Conversation

@denik

@denikdenik commented Dec 5, 2024

Copy link
Copy Markdown
Contributor

Changes

Remove two duplicate implementations of the same logic, switch everywhere to folders.FindDirWithLeaf.

Add Abs() call to FindDirWithLeaf, it cannot really work on relative paths.

Tests

Existing tests.

The git module now uses alternative implementation that uses os module + strings as path directly (#1945).
The remaining use case is also updated to use git.FindLeafInTree, as it's a better fit.
@denik
denikforce-pushed the DECO-24125--cleanup-vfs-leaf branch from c197c74 to 2e102b7CompareDecember 9, 2024 16:02

@pieternpietern left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@denik I forgot: we have this function implemented exactly under libs/folders. You could use that instead of a separate copy. We use this as well to find the bundle root (looking for a databricks.yml file).

@denikdenik changed the title Clean up vfs/leaf.goSwitch to folders.FindDirWithLeafDec 10, 2024
@denik

Copy link
Copy Markdown
ContributorAuthor

@denik I forgot: we have this function implemented exactly under libs/folders. You could use that instead of a separate copy. We use this as well to find the bundle root (looking for a databricks.yml file).

Good catch! Switched everything to use that.

@pieternpietern changed the title Switch to folders.FindDirWithLeafSwitch to folders.FindDirWithLeafDec 10, 2024

@pieternpietern left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you!

@pietern

Copy link
Copy Markdown
Contributor

(I Markdown-ified the title to show up nicely in the GitHub release notes)

@github-actions

Copy link
Copy Markdown
Contributor

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/cli

Inputs:

  • PR number: 1963
  • Commit SHA: 1e7d06cc3e1cd0c28b1d0e83cc7f0a52f18f4311

Checks will be approved automatically on success.

@eng-dev-ecosystem-bot

Copy link
Copy Markdown
Collaborator

Test Details: go/deco-tests/12259236623

@denik

Copy link
Copy Markdown
ContributorAuthor

Integration test failure is unrelated (TestAccBundleDeployUcSchema)

@denik
denik merged commit 4236e71 into mainDec 11, 2024
@denik
denik deleted the DECO-24125--cleanup-vfs-leaf branch December 11, 2024 08:44
denik added a commit that referenced this pull request May 20, 2026
## Changes
Remove two duplicate implementations of the same logic, switch
everywhere to folders.FindDirWithLeaf.
Add Abs() call to FindDirWithLeaf, it cannot really work on relative
paths.
## Tests
Existing tests.
Sign up for freeto 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.

5 participants

@denik@pietern@eng-dev-ecosystem-bot@andrewnester@shreyas-goenka