Added TFT Endpoints : Issue#640 #645
Conversation
|
Thank you for you first contribution! The code looks good 👍 Please create new endpoints for the added methods, i.e. add:
with the corresponding interfaces, so that the endpoint classes match the API sections in Riot's documentation. |
those endpoints.Also, moved unit tests and created classes for tft endpoints to ensure maintainability
|
Sounds good. I'll have those changes made pushed tomorrow morning. If you have any other issues I can work on let me know! |
|
Hello, I'm sorry didn't saw this pull request and worked last few theys on my changes. I hope I added something new 👍 |
…dpoint and ISummonerEndpoint to preserve file history.
| /// </summary> | ||
| [JsonProperty("summonerLevel")] | ||
| public long Level { get; set; } | ||
| /// <summary> |
There was a problem hiding this comment.
🎨 missing an empty line
additionally, how is this decoded?
|
Fixed the issues pointed out by @BenFradet in latest commit. |
|
There was a problem hiding this comment.
Please add initializations for the endpoints in the RiotApi constructors here and below.
| /// <param name="region">Region in which the summoner is.</param> | ||
| /// <param name="puuid"></param> | ||
| /// <returns>A list of strings</returns> | ||
| Task<List<string>> GetTftMatchIdsByPuuidAsync(Region region, string puuid, int count = 20); |
There was a problem hiding this comment.
Can you remove ByPuuid from the naming to match the naming of the match endpoint.
| /// <param name="region">Region in which the summoner is.</param> | ||
| /// <param name="matchId">The match id for the match wanting to be retrieved</param> | ||
| /// <returns><see cref="Match"> object </returns> | ||
| Task<TftMatch> GetTftMatchByIdAsync(Region region, string matchId); |
| } | ||
|
|
||
| return match; | ||
|
|
|
|
||
| namespace RiotSharp.Endpoints.TftMatchEndpoint | ||
| { | ||
| public class TftMatch |
There was a problem hiding this comment.
This class does not match the schema of Riot's api resonse. You might have forgotten to include your changes for this one.
|
Hello, is this working? Can I put it on vb.net? Here is giving error (tftmatchendpoint) |
|
Is there a timeline for merging and deploying these changes? I just about implemented the whole TFT API until I saw it's already in this PR. This would be very useful for a project I'm working on, thanks! |
Added the endpoints needed for tft. Endpoints were added for League, Match, and Summoner.
Also, added unit tests for the endpoints (minus match endpoints)