bagetter / BaGetter

A lightweight NuGet and symbol server
https://www.bagetter.com
MIT License
243 stars 53 forks source link

Fixed tags parsing for metadata #91

Closed Splamy closed 7 months ago

Splamy commented 7 months ago

Summary of the changes (in less than 80 chars)

Addresses https://github.com/bagetter/BaGetter/issues/89

Annotations

It felt weird in the unit test to check the deserialized value of the tags to be [ "json bson serializer" ], but I don't think tag normalizing should happen in the JsonConveter. The tags will be properly split up in the V3UpstreamClient.ToPackage method.

I had to add .Replace("\r\n", "\n") in the unit test json writer otherwise all related tests would fail. It seems that the strings in the tests are declared with \n even though the .editorconfig defines \r\n as the project line endings. The github actions will probably be default run linux, but i'm on a windows machine so it apparently wrote the json with crlf. You could maybe consider checking json for structural equality instead of string-wise like this https://github.com/Splamy/JsonBinMin/blob/main/JsonBinMin.Tests/AssertUtil.cs#L17-L57 or if you find a good library.

FroggieFrog commented 7 months ago

Is this the same as PR #85 and PR #90 ?

Splamy commented 7 months ago

Huh, yeah, seems like you were faster than me 😅

The only thing I have additionally is the NormalizeTags method, since otherwise you would return [ "tag1 tag2" ] from an upstream mirror like github Feel free to close this if it conflicts and just pick the snippet you need.

FroggieFrog commented 7 months ago

Update: Everything should be included in PR #90.

I will merge your additions/extensions (incl. tests) into PR #90 . Thank you!

Splamy commented 7 months ago

Thanks! And sorry for the duplication trouble, shoud've checked the open pr's, I really just didn't except you to tackle my issue so quickly 😉

seriouz commented 7 months ago

@FroggieFrog @Splamy Thank you, you two :)