-
Notifications
You must be signed in to change notification settings - Fork 129
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
API module: The Phantom Menace #536
Comments
+1 for removing the bangified methods. The only use case I can see for them is if you want some process to die if the function fails. That's simple enough for a user to do by themselves, and the benefit of being able to clean all of this up would be welcome. I think it's a bad idea (probably), but want to point out the possibility of putting these (and other) functions in broader context modules. Think like As for the constants module, I'd opt to leave it as is. There's a lot of things in there that are scoped such that they wouldn't fit under any of the suggested modules, and I'm not sure there's a lot to be gained from such an internal facing module. Good stuff! |
I like it. I second the suggestion to move functions into I think I saw mention of using delegates for this in the discord, which I think is a good idea and, imo, will help keep the API module clean. (Edit: to be clear I think retaining the existing API module and using it to delegate to the implementations is the right play, as it prevents this being a breaking change. The API module can later be deprecated in a major release) |
I am in favour of these as well. I agree delegates are probably a good idea to keep it compatible for now. One thought I have. Should we change |
Agree on this, that is more idiomatic, I'll rewrite later today. |
I do think to save the hellish possibility of 4k+ lines of new code as well as maintaining a huge api.ex file we really should just drop these methods for 1.0, but I'm open to looking more into the delegates. |
Not sure if anyone is working on this, but I'd love to take a stab at it if nobody else is. |
When I open
api.ex
, my Emacs lags. This is simply unacceptable, as Emacs is known to be blazingly fast. I pondered, why could this be, why would my Emacs have anything above mere milliseconds of delay on opening a file. Then it hit me.I have decided that this will not do, and that we must scope out all methods to respective sub-modules. In the words of Craig:
I have hence decided to form an opinionated list of how we should do this restructure to kick off the discussion process, I encourage as much feedback as we can get here. I used
Nostrum.Api.__info__(:functions) |> Keyword.keys |> Enum.uniq
to get this list.The convention I have used below is the newly scoped module, then the current name (under
Nostrum.Api
) and the post-move module name.Thoughts
Migration
Would be very cool to try make this migration as simple as possible, maybe looking at custom Credo checks or other utilities to scan for usages of old
Nostrum.Api
calls or aliases and recommending the changes.Bangified methods
At the same time we make this change, we should ensure that all methods have a bangified equivalent if they can potentially return an API error.
I also wish there was a better way to do bangified functions, or to offload that to the user, because it makes the API functions list twice as complicated as it needs to be. In my mind, I want to shove
bangify
to the user-side and let them upgrade things to exceptions, but I don't know if that's as idiomatic/a good idea.Thoughts on this greatly appreciated, I wish we could do away with them, (and frankly, we already are missing enough of them that I think users would find ways to just handle things better)
Food for thought: Constants
I would also like to ask the question of how we handle the
Constants
module and if that also requires any form of refactoring after these changes.Guild module
Slightly related, but since the proposed Guild module is huge, I'd like to suggest:
Nostrum.Api.Expressions
, like the API calls it)create_guild_integrations
, it's not even a documented routeOther notes
I have only looked at public functions in the
Api
module, there are a lot of private internals that would need to be figured out.The Modules
Nostrum.Api
request
=>request
request_multipart
=>request_multipart
Nostrum.Api.ApplicationCommands
batch_edit_application_command_permissions
=>batch_edit_permissions
bulk_overwrite_global_application_commands
=>bulk_overwrite_global_commands
bulk_overwrite_guild_application_commands
=>bulk_overwrite_guild_commands
create_global_application_command
=>create_global_command
create_guild_application_command
=>create_guild_command
delete_global_application_command
=>delete_global_command
delete_guild_application_command
=>delete_guild_command
edit_application_command_permissions
=>edit_command_permissions
edit_global_application_command
=>edit_global_command
edit_guild_application_command
=>edit_guild_command
get_application_command_permissions
=>get_command_permissions
get_global_application_commands
=>get_global_commands
get_guild_application_command_permissions
=>get_guild_command_permissions
get_guild_application_commands
=>get_guild_commands
Nostrum.Api.AutoModeration
create_guild_auto_moderation_rule
=>create_auto_moderation_rule
delete_guild_auto_moderation_rule
=>delete_auto_moderation_rule
get_guild_auto_moderation_rule
=>get_auto_moderation_rule
get_guild_auto_moderation_rules
=>get_auto_moderation_rules
modify_guild_auto_moderation_rule
=>modify_auto_moderation_rule
Nostrum.Api.Channel
add_pinned_channel_message
=>pin
bulk_delete_messages
=>bulk_delete_messages
create_guild_channel
=>create
delete_channel
=>delete
delete_channel_permissions
=>delete_permissions
delete_pinned_channel_message
=>unpin
edit_channel_permissions
=>edit_permissions
get_channel
=>get
get_channel_messages
=>get_messages
get_channel_webhooks
=>get_webhooks
get_pinned_messages
=>get_pins
modify_channel
=>modify
start_typing
=>start_typing
Nostrum.Api.Guild
add_guild_member
=>add_member
begin_guild_prune
=>begin_prune
create_guild_ban
=>ban_member
create_guild_emoji
=>create_emoji
create_guild_integrations
=>create_integration
delete_guild
=>delete
delete_guild_emoji
=>delete_emoji
delete_guild_integrations
=>delete_integration
get_guild
=>get
get_guild_audit_log
=>get_audit_log
get_guild_ban
=>get_ban
get_guild_bans
=>get_bans
get_guild_channels
=>get_channels
get_guild_emoji
=>get_emoji
get_guild_integrations
=>get_guild_integrations
get_guild_member
=>get_member
get_guild_prune_count
=>estimate_prune_count
get_guild_roles
=>get_roles
get_scheduled_events
=>get_scheduled_events
get_guild_webhooks
=>get_webhooks
get_guild_widget
=>get_widget
get_voice_region
=>get_voice_region
leave_guild
=>leave
list_guild_emojis
=>list_guild_emojis
list_guild_members
=>get_members
list_voice_regions
=>list_voice_regions
modify_current_user_nick
=>modify_self_nick
modify_guild
=>modify
modify_guild_channel_positions
=>modify_channel_positions
modify_guild_emoji
=>modify_emoji
modify_guild_integrations
=>modify_integrations
modify_guild_member
=>modify_member
modify_guild_role_positions
=>modify_role_positions
modify_guild_widget
=>modify_widget
remove_guild_ban
=>unban_member
remove_guild_member
=>kick_member
sync_guild_integrations
=>sync_integrations
Nostrum.Api.Interactions
create_followup_message
=>create_followup_message
create_interaction_response
=>create_response
delete_interaction_followup_message
=>delete_followup_message
delete_interaction_response
=>delete_response
edit_interaction_response
=>edit_response
get_original_interaction_response
=>get_original_response
Nostrum.Api.Invites
get_invite
=>get
delete_invite
=>delete_invite
get_guild_invites
=>get_guild_invites
create_channel_invite
=>create
get_channel_invites
=>get_channel_invites
Nostrum.Api.Message
create_message
=>create
create_reaction
=>react
delete_all_reactions
=>clear_reactions
delete_message
=>delete
delete_own_reaction
=>unreact
delete_reaction
=>delete_reaction
delete_user_reaction
=>delete_user_reaction
edit_message
=>edit
get_channel_message
=>get
get_reactions
=>get_reactions
Nostrum.Api.Polls
expire_poll
=>expire
get_poll_answer_voters
=>get_answer_voters
Nostrum.Api.Role
add_guild_member_role
=>add_member
create_guild_role
=>create
delete_guild_role
=>delete
modify_guild_role
=>modify
remove_guild_member_role
=>remove_member
Nostrum.Api.ScheduledEvent
create_guild_scheduled_event
=>create
delete_guild_scheduled_event
=>delete
get_guild_scheduled_event
=>get
get_guild_scheduled_event_users
=>get_users
modify_guild_scheduled_event
=>modify
Nostrum.Api.Self
get_application_information
=>get_application_information
get_current_user
=>get
get_token
=>get_token
get_user_dms
=>get_dms
modify_current_user
=>modify
update_shard_status
=>update_shard_status
update_status
=>update_status
update_voice_status
=>update_voice_status
Nostrum.Api.Thread
add_thread_member
=>add_member
get_thread_member
=>get_member
get_thread_members
=>get_members
join_thread
=>join
leave_thread
=>leave
list_guild_threads
=>list_threads
list_joined_private_archived_threads
=>list_joined_private_archived_threads
list_private_archived_threads
=>list_private_archived_threads
list_public_archived_threads
=>list_public_archived_threads
remove_thread_member
=>remove_member
start_thread
=>create
start_thread_in_forum_channel
=>create_in_forum
start_thread_with_message
=>create_with_message
Nostrum.Api.User
create_dm
=>create_dm
create_group_dm
=>create_group_dm
get_current_user_guilds
=>get_current_user_guilds
get_user
=>get
get_user_connections
=>get_user_connections
Nostrum.Api.Webhook
create_webhook
=>create
delete_webhook
=>delete
edit_webhook_message
=>edit_message
execute_git_webhook
=>execute_git
execute_slack_webhook
=>execute_slack
execute_webhook
=>execute
get_webhook
=>get
get_webhook_message
=>get_message
get_webhook_with_token
=>get_with_token
modify_webhook
=>modify
modify_webhook_with_token
=>modify_with_token
The text was updated successfully, but these errors were encountered: