Add Protocol Urls #103

Merged
Chiggy-Playz merged 17 commits from ProtocolUrls into 2.0 2021-10-27 12:32:50 +00:00
Chiggy-Playz commented 2021-10-16 19:23:37 +00:00 (Migrated from github.com)

Summary

Adds support for discord:// urls.

Checklist

  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • This PR adds something new (e.g. new method or parameters).
  • This PR fixes an issue.
  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • This PR is not a code change (e.g. documentation, README, ...)
## Summary Adds support for `discord://` urls. ## Checklist - [x] If code changes were made then they have been tested. - [x] I have updated the documentation to reflect the changes. - [x] This PR adds something new (e.g. new method or parameters). - [ ] This PR fixes an issue. - [ ] This PR is a breaking change (e.g. methods or parameters removed/renamed) - [ ] This PR is **not** a code change (e.g. documentation, README, ...)
ccev commented 2021-10-18 06:03:27 +00:00 (Migrated from github.com)

For the last 6 protocol URLs, i think it would be better to add a new property for the relevant objects that returns the formatted url.

Also, aren't Enum names usually uppercase?

For the last 6 protocol URLs, i think it would be better to add a new property for the relevant objects that returns the formatted url. Also, aren't Enum names usually uppercase?
StockerMC (Migrated from github.com) reviewed 2021-10-18 14:12:26 +00:00
StockerMC (Migrated from github.com) commented 2021-10-18 14:12:26 +00:00
    def format(self, **kwargs: Any) -> str:
```suggestion def format(self, **kwargs: Any) -> str: ```
StockerMC (Migrated from github.com) reviewed 2021-10-18 14:21:13 +00:00
StockerMC (Migrated from github.com) commented 2021-10-18 14:21:13 +00:00

This should be updated to be

url: Optional[Union[:class:`str`, :class:`ProtocolUrl`]]

and the typehint should be updated as well.

This should be updated to be ``` url: Optional[Union[:class:`str`, :class:`ProtocolUrl`]] ``` and the typehint should be updated as well.
Chiggy-Playz (Migrated from github.com) reviewed 2021-10-18 14:22:09 +00:00
Chiggy-Playz (Migrated from github.com) commented 2021-10-18 14:22:09 +00:00
I agree, but, well. Gnome. https://discord.com/channels/514232441498763279/881248749849026590/899009786077659237
StockerMC (Migrated from github.com) reviewed 2021-10-18 14:25:49 +00:00
StockerMC (Migrated from github.com) commented 2021-10-18 14:25:49 +00:00

Ah okay. The typehint should still be updated though.

Ah okay. The typehint should still be updated though.
StockerMC (Migrated from github.com) reviewed 2021-10-18 14:26:58 +00:00
StockerMC (Migrated from github.com) commented 2021-10-18 14:26:57 +00:00

Now that I think about it, the typehint should probably be Optional[Any] because it gets casted to str, same as content in abc.Messageable.send.

Now that I think about it, the typehint should probably be `Optional[Any]` because it gets casted to `str`, same as `content` in `abc.Messageable.send`.
Chiggy-Playz (Migrated from github.com) reviewed 2021-10-18 14:28:11 +00:00
Chiggy-Playz (Migrated from github.com) commented 2021-10-18 14:28:11 +00:00

Shouldn't both be updated though? Changing the typehint, but not update docs sounds inconsistent? That is why I did not change typehint

Shouldn't both be updated though? Changing the typehint, but not update docs sounds inconsistent? That is why I did not change typehint
Chiggy-Playz (Migrated from github.com) reviewed 2021-10-18 14:34:06 +00:00
Chiggy-Playz (Migrated from github.com) commented 2021-10-18 14:34:06 +00:00

Oh ok.

Oh ok.
Chiggy-Playz (Migrated from github.com) reviewed 2021-10-18 14:40:54 +00:00
Chiggy-Playz (Migrated from github.com) commented 2021-10-18 14:40:53 +00:00

Gonna be honest, I don't understand typing stuff so much, so if you want you can create a suggestion for this.

Gonna be honest, I don't understand typing stuff so much, so if you want you can create a suggestion for this.
Gnome-py (Migrated from github.com) reviewed 2021-10-22 17:17:32 +00:00
Gnome-py (Migrated from github.com) commented 2021-10-22 17:17:32 +00:00
    guild_create = "discord://-/guilds/create"
```suggestion guild_create = "discord://-/guilds/create" ```
Gnome-py (Migrated from github.com) reviewed 2021-10-22 17:18:54 +00:00
Gnome-py (Migrated from github.com) commented 2021-10-22 17:18:53 +00:00

Comment should be moved up, so you don't need the 3 lines for one definition

Comment should be moved up, so you don't need the 3 lines for one definition
Chiggy-Playz (Migrated from github.com) reviewed 2021-10-22 17:23:31 +00:00
Chiggy-Playz (Migrated from github.com) commented 2021-10-22 17:23:31 +00:00

Resolved with eadd424

Resolved with eadd424
Sign in to join this conversation.
No description provided.