Skip to content
This repository has been archived by the owner on Mar 18, 2024. It is now read-only.

fix(packagediff): fix packages being built when the path to package start with same structure #1423

Merged

Conversation

azlam-abdulsalam
Copy link
Contributor

@azlam-abdulsalam azlam-abdulsalam commented Oct 13, 2023

paths are matched with includes rather than checking the directory properly. This can result in
packages being built if the directory structure is similar

fixes #1396

Summary generated by Reviewpad on 13 Oct 23 11:41 UTC

This pull request fixes an issue where packages are built incorrectly when the path to the package starts with the same structure. Instead of properly checking the directory, paths were matched using includes, which could result in packages being built even if the directory structure was only similar. The patch modifies the PackageDiffImpl.ts file to properly check the directory structure before building packages. Additionally, a new test case is added in the PackageDiffImpl.test.ts file to verify the fix.

Checklist

All items have to be completed before a PR is merged

  • Adhere to Contribution Guidelines
  • Updates to Decision Records considered?
  • Updates to documentation at DX@Scale Guide considered?
  • Tested changes?
  • Unit Tests new and existing passing locally?

…tart with same structure

paths are matched with includes rather than checking the directory properly. This can result in
packages being built if the directory structure is similar

fixes #1396
@azlam-abdulsalam azlam-abdulsalam linked an issue Oct 13, 2023 that may be closed by this pull request
@reviewpad reviewpad bot added small Pull request is small waiting-for-review labels Oct 13, 2023
@codecov
Copy link

codecov bot commented Oct 13, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (7c5d9d4) 46.80% compared to head (e336f46) 46.86%.

❗ Current head e336f46 differs from pull request most recent head 38fdf45. Consider uploading reports for the commit 38fdf45 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1423      +/-   ##
==========================================
+ Coverage   46.80%   46.86%   +0.05%     
==========================================
  Files          70       70              
  Lines        2662     2665       +3     
  Branches      302      302              
==========================================
+ Hits         1246     1249       +3     
  Misses       1414     1414              
  Partials        2        2              
Files Coverage Δ
packages/core/src/package/diff/PackageDiffImpl.ts 86.66% <100.00%> (+0.55%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@azlam-abdulsalam azlam-abdulsalam merged commit 1d31cd3 into main Oct 17, 2023
4 of 5 checks passed
@azlam-abdulsalam azlam-abdulsalam deleted the 1396-similarly-named-packages-are-built-unnecessarily branch October 17, 2023 01:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
small Pull request is small waiting-for-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Similarly-named packages are built unnecessarily
2 participants