feat: adapt to v1 - #33
Conversation
fe4d82f to
cf0b1f4
Compare
|
README updated. |
6f81889 to
a36bd10
Compare
|
looks good to me |
Updated comments and improved formatting in DiscordBotListApi class.
velddev
left a comment
velddev
left a comment
There was a problem hiding this comment.
overall looks good, some comment to standardize it a bit more to C# defaults 👍
| namespace DiscordBotsList.Api.Data | ||
| { | ||
| /// <summary> | ||
| /// A project's platform. |
There was a problem hiding this comment.
nit; you don't need the indentation in docstrings here
| internal class PlatformConverter : JsonConverter<Platform> | ||
| { | ||
| public override Platform Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) | ||
| { | ||
| if (reader.TokenType == JsonTokenType.String) | ||
| { | ||
| switch (reader.GetString()) | ||
| { | ||
| case "discord": return Platform.Discord; | ||
| } | ||
| } | ||
|
|
||
| throw new InvalidOperationException(); | ||
| } | ||
|
|
||
| public override void Write(Utf8JsonWriter writer, Platform platform, JsonSerializerOptions options) => throw new InvalidOperationException(); | ||
| } |
There was a problem hiding this comment.
| public enum ProjectType | ||
| { | ||
| DiscordBot, | ||
| DiscordServer | ||
| } |
There was a problem hiding this comment.
These do not mean "Discord" that's what platform is for. You could have a Slack bot or a Guilded Server.
| internal class ProjectTypeConverter : JsonConverter<ProjectType> | ||
| { | ||
| public override ProjectType Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) | ||
| { | ||
| if (reader.TokenType == JsonTokenType.String) | ||
| { | ||
| switch (reader.GetString()) | ||
| { | ||
| case "bot": return ProjectType.DiscordBot; | ||
| case "server": return ProjectType.DiscordServer; | ||
| } | ||
| } | ||
|
|
||
| throw new InvalidOperationException(); | ||
| } | ||
|
|
||
| public override void Write(Utf8JsonWriter writer, ProjectType type, JsonSerializerOptions options) => throw new InvalidOperationException(); | ||
| } |
There was a problem hiding this comment.
same as above; use enum converter
| /// <summary> | ||
| /// The project's total vote count. | ||
| /// </summary> | ||
| [JsonPropertyName("votes_total")] |
There was a problem hiding this comment.
Instead of naming all these you can set the jsonSerializerSettings to use a snakeCase Casing. https://learn.microsoft.com/en-us/dotnet/api/system.text.json.jsonnamingpolicy?view=net-10.0
| using System.Text.Json.Serialization; | ||
|
|
||
| namespace DiscordBotsList.Api.Internal | ||
| namespace DiscordBotsList.Api.Serialization |
There was a problem hiding this comment.
Since this is a breaking change, could we rename it to Topgg.Api.XYZ?
There was a problem hiding this comment.
Sure! I'm renaming the API wrapper to Topgg.Sdk.Api and the Webhooks library to Topgg.Sdk.Webhooks.
3 participants
Closes #28
The following pull request is a toned down version of #29. This pull request focuses solely on adding support for v1, like posting bot commands, getting vote information, and widgets.
Other than that, it also adds error handling for invalid status codes by using
response.EnsureSuccessStatusCode().This pull request depends on #32.It has been tested.