Skip to content
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

feat(slashcommands): add JSON objects along with builders #1149

Closed

Conversation

megatank58
Copy link
Contributor

Please describe the changes this PR makes and why it should be merged:

#1122 Adding JSON objects as an alternative option to builders will allow advanced developers to copy paste understand code quickly and something that is more native to them.

One question is whether the builders page should also have the JSON objects for direct comparison with builders or should they not have them as the page is specifically made for builders?

@BenjammingKirby
Copy link
Contributor

I also disagree with having this in the first place, we don't need a raw json example for everything, that'd be ass to maintain and just unneeded stuff cluttering the guide imo. What I would rather do is recommend builders and link to the api docs and perhaps mention discord-api-types for typescript users. In the case of the "advanced users", that should be enough for them.

Copy link
Member

@almostSouji almostSouji left a comment

Choose a reason for hiding this comment

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

Prefer enums over magic numbers, if you really feel the need to, you can note the value as a comment.

@megatank58
Copy link
Contributor Author

Switched to enums

guide/interactions/slash-commands.md Outdated Show resolved Hide resolved
@almostSouji almostSouji marked this pull request as draft October 17, 2022 10:17
@almostSouji
Copy link
Member

This will need to be revisited after the semantic and structural rewrite that's currently in progress

@almostSouji almostSouji added the blocked Blocked by other PR or upstream label Oct 17, 2022
@megatank58
Copy link
Contributor Author

It's probably better to remake this pr entirely (or rethink the idea as a whole) as the codebase has changed a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Blocked by other PR or upstream type: enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants