Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minimatch for directory exclusion #1091

Conversation

chriskarlsson
Copy link

@chriskarlsson chriskarlsson commented Apr 29, 2024

To me the description on how to define DirectoryExclusionList was very confusing since the comparison was globing and not minimatch and that there was no mention of ';' being used to define multiple patterns. This PR aims to solve both of those issues.

The minimatch implementation is a port of https://github.com/isaacs/minimatch to C#.

@chriskarlsson chriskarlsson requested a review from a team as a code owner April 29, 2024 15:22
@chriskarlsson
Copy link
Author

@microsoft-github-policy-service agree company="Appgate"

@chriskarlsson chriskarlsson force-pushed the chriskarlsson/minimatch-on-directory-exclusion branch from fb47a50 to 18c374a Compare May 2, 2024 06:47
Copy link

codecov bot commented May 2, 2024

Codecov Report

Attention: Patch coverage is 88.03828% with 50 lines in your changes are missing coverage. Please review.

Project coverage is 76.1%. Comparing base (5894c27) to head (e0161c9).
Report is 10 commits behind head on main.

Files Patch % Lines
...osoft.ComponentDetection.Orchestrator/Minimatch.cs 87.9% 29 Missing and 21 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main   #1091     +/-   ##
=======================================
+ Coverage   75.7%   76.1%   +0.4%     
=======================================
  Files        245     246      +1     
  Lines      10972   11378    +406     
  Branches    1101    1174     +73     
=======================================
+ Hits        8307    8663    +356     
- Misses      2343    2372     +29     
- Partials     322     343     +21     

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

@cobya cobya added version:major type:feature Feature (new functionality) breaking change Breaking change, requires major version bump .NET Pull requests that update .net code labels May 13, 2024
Copy link
Contributor

@cobya cobya left a comment

Choose a reason for hiding this comment

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

First of all, thank you for your contribution to the repo! There are some things that I've noted which would be needed before we're able to accept the contribution 👍

Since this would be a breaking change in how our directory exclusion pattern works for downstream consumers, I've tagged it for the next major release of Component Detection.

Do you have a timeline when this is needed for your tooling? Generally speaking we try and give a few weeks heads before pushing major versions.

@cobya cobya added the status:waiting-on-response Waiting on a response/more information from the user label May 13, 2024
@chriskarlsson
Copy link
Author

Sorry. I've been quite busy the last weeks. I hope I could get back to it this week.

@chriskarlsson
Copy link
Author

Do you have a timeline when this is needed for your tooling? Generally speaking we try and give a few weeks heads before pushing major versions.

Missed your question before. I've worked around this in our tooling. So for us this isn't necessary. Just thought it would be a nice upgrade to the repo.

* License attribution
* RegexOptions.Compiled
* Better exception message
* Better test coverage
@chriskarlsson chriskarlsson force-pushed the chriskarlsson/minimatch-on-directory-exclusion branch from 18c374a to 5805006 Compare May 16, 2024 08:52
@chriskarlsson
Copy link
Author

@cobya - Updated with respect to your comments last week. Please have a look to see if it looks good to you.

@chriskarlsson chriskarlsson requested a review from cobya May 28, 2024 07:19
@cobya cobya changed the base branch from main to cobya/v5 May 28, 2024 16:50
@cobya cobya merged commit e52ef78 into microsoft:cobya/v5 May 28, 2024
20 of 23 checks passed
@cobya
Copy link
Contributor

cobya commented May 28, 2024

Thanks again for the contribution here 🫶 I've merged the changes into the cobya/v5 branch will be used as the baseline for the upcoming breaking changes tracked in #1135 and the next major release!

@cobya cobya removed the status:waiting-on-response Waiting on a response/more information from the user label May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Breaking change, requires major version bump .NET Pull requests that update .net code type:feature Feature (new functionality) version:major
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants