Skip to content

Comments

Add maxcount parameter to SteamDirectory.LoadAsync#577

Merged
yaakov-h merged 6 commits intomasterfrom
560-steamdirectory-madcount
Jul 22, 2018
Merged

Add maxcount parameter to SteamDirectory.LoadAsync#577
yaakov-h merged 6 commits intomasterfrom
560-steamdirectory-madcount

Conversation

@yaakov-h
Copy link
Member

@yaakov-h yaakov-h commented Jul 21, 2018

Closes #560.

cc @xPaw

@yaakov-h yaakov-h added this to the 2.2.0 milestone Jul 21, 2018
@xPaw
Copy link
Member

xPaw commented Jul 21, 2018

No (SteamConfiguration configuration, int maxNumServers) overload?

xPaw
xPaw reviewed Jul 21, 2018
View reviewed changes
SteamKit2/SteamKit2/Steam/WebAPI/SteamDirectory.cs Outdated

if ( maxNumServers.HasValue )
{
args[ "maxcount" ] = maxNumServers.Value.ToString(CultureInfo.InvariantCulture);
Copy link
Member

@xPaw xPaw Jul 21, 2018

Choose a reason for hiding this comment

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

@codecov-io
Copy link

codecov-io commented Jul 21, 2018 •
edited
Loading

Codecov Report

Merging #577 into master will increase coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #577      +/-   ##
==========================================
+ Coverage   22.12%   22.12%   +<.01%     
==========================================
  Files          86       86              
  Lines        8666     8655      -11     
  Branches      714      715       +1     
==========================================
- Hits         1917     1915       -2     
+ Misses       6624     6613      -11     
- Partials      125      127       +2
Impacted Files Coverage Δ
SteamKit2/SteamKit2/Steam/WebAPI/SteamDirectory.cs 0% <0%> (ø) ⬆️
SteamKit2/SteamKit2/Steam/WebAPI/WebAPI.cs 36.27% <0%> (-0.5%) ⬇️
SteamKit2/SteamKit2/Types/SteamID.cs 79.42% <0%> (-0.37%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5dba62d...7eb45ed. Read the comment docs.

@yaakov-h
Copy link
Member Author

yaakov-h commented Jul 21, 2018

I did consider it, but with C# 7.1 you can easily call .LoadAsync(configuration, maxNumServers, default). I don't feel there's much point adding an additional overload that just passes through a default(CancellationToken)/CancellationToken.None.

voided
voided approved these changes Jul 21, 2018
View reviewed changes
SteamKit2/SteamKit2/Steam/WebAPI/SteamDirectory.cs

var task = directory.CallAsync( HttpMethod.Get, "GetCMList", version: 1, args: args );
return task.ContinueWith( t =>
var response = await directory.CallAsync( HttpMethod.Get, "GetCMList", version: 1, args: args ).ConfigureAwait( false );
Copy link
Member

@voided voided Jul 21, 2018

Choose a reason for hiding this comment

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

Shame we can't pass the cancellation token further. Maybe worth expanding WebAPI's AsyncInterface someday.

@yaakov-h yaakov-h merged commit c26ac89 into master Jul 22, 2018
@yaakov-h yaakov-h deleted the 560-steamdirectory-madcount branch July 22, 2018 01:17
This was referenced Aug 5, 2018
Open
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xPaw xPaw xPaw left review comments

@voided voided voided approved these changes

Assignees

No one assigned

Labels

enhancement

Projects

None yet

Milestone

2.2.0

Development

Successfully merging this pull request may close these issues.

4 participants

@yaakov-h @xPaw @codecov-io @voided