-
Notifications
You must be signed in to change notification settings - Fork 317
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
add support for GitHub / Azure Artefacts #602
add support for GitHub / Azure Artefacts #602
Conversation
…ates and getting single NuGet packages using registrationsBaseUrl
searchQueryBuilder.Append(' '); | ||
} | ||
|
||
searchQueryBuilder.Append($"packageid:{packagesToFetch[i].Id}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you are using string interpolation here then perhaps a cleaner solution instead of StringBuilder is to just build a List and then use string.Join(" ", list)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would need:
var query = string.Join(' ', packagesToFetch.Skip(i).Take(UpdateSearchBatchSize).Select(package => $"{package.Id}_"));
i += UpdateSearchBatchSize;
As there is no Chunk method in older C# we need the index. Not sure if the Unity implementation of the Skip
already has optimization for List
. Do you think it would make the code more readable, with the Skip / Take?
Another think is we could use AppendFormat
as none .Net Core doesn't has the Append
overload that takes a AppendInterpolatedStringHandler
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just thought to replace StringBuilder with List and ToString with string.join leaving everything else the same.
registrationsBaseUrl
registrationsBaseUrl
fixes #598
and fixes #589