Add Properties VCPKG #855
Add Properties VCPKG #855
Conversation
src/Microsoft.ComponentDetection.Contracts/TypedComponent/VcpkgComponent.cs
Outdated
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## main #855 +/- ##
=====================================
Coverage 77.5% 77.5%
=====================================
Files 235 235
Lines 9936 9953 +17
=====================================
+ Hits 7702 7719 +17
Misses 2234 2234
|
|
👋 Hi! It looks like you modified some files in the
If none of the above scenarios apply, feel free to ignore this comment 🙂 |
|
|
||
| if (!string.IsNullOrEmpty(locationArr[3])) | ||
| { | ||
| this.GitRepositoryName = locationArr[3].TakeWhile(ch => char.IsLetterOrDigit(ch)).ToString(); |
There was a problem hiding this comment.
I think this check may fail for repositories with valid names like component-detection
| * "git+https://github.com/Microsoft/vcpkg#ports/nlohmann-json" | ||
| */ | ||
| var locationArr = this.DownloadLocation.Split('/'); | ||
| if (!string.IsNullOrEmpty(locationArr[2])) |
There was a problem hiding this comment.
Let's do an array length check, in which we expect at least 4 fragments, if less than that this should be noop. Otherwise we can hit index out of range exceptions here.
| * "git+https://github.com/leethomason/tinyxml2@9.0.0" | ||
| * "git+https://github.com/Microsoft/vcpkg#ports/nlohmann-json" | ||
| */ | ||
| var locationArr = this.DownloadLocation.Split('/'); |
There was a problem hiding this comment.
qq, if downloadlocation is null will the split work?
There was a problem hiding this comment.
I have a null or empty check before this method gets called
|
|
||
| using System.Linq; | ||
| using PackageUrl; | ||
|
|
||
| public class VcpkgComponent : TypedComponent |
There was a problem hiding this comment.
I expect to see some tests to validate these new changes.
Added GitRepositoryOwner and GitRepositoryName to prevent fuzzy matching