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: add `userAgent` to `RESTOptions` #4

Closed
wants to merge 2 commits into from

Conversation

@tjrgg
Copy link
Contributor

tjrgg commented Mar 8, 2020

I've added userAgent to RESTOptions to allow for a custom User-Agent. By default, the option uses the one that was defined, but I've defined it in the RESTOptions and removed the export, since it was no longer necessary.

There probably isn't a lot of uses for this, but I didn't want to see this ability taken away by using this package in a package not associated with Dirigeants or Project Blue.

@PyroTechniac

This comment has been minimized.

Copy link

PyroTechniac commented Mar 8, 2020

The User Agent, as per Discord API Documentation, is meant to give discord information about the client library being used, and should not be changed by end users.

@tjrgg

This comment has been minimized.

Copy link
Contributor Author

tjrgg commented Mar 8, 2020

My use case for having this option would be to use this package as part of another client library. Since this is built as a separate package, it is possible that developers like myself may choose to use this package as part of other client libraries and/or frameworks, not just in association with Project Blue. As such, I don't think it makes sense to identify those other libraries to Discord as this project. Given this possibility, I don't think it makes sense to restrict the ability for changing the User-Agent.

@MrJacz
MrJacz approved these changes Mar 8, 2020
Copy link
Member

MrJacz left a comment

LGTM

Copy link

cfanoulis left a comment

just a formality from discord

@@ -128,7 +129,7 @@ export class RESTManager {

// Assign the basic request headers
const headers: Headers = {
'User-Agent': UserAgent,
'User-Agent': this.options.userAgent,

This comment has been minimized.

Copy link
@cfanoulis

cfanoulis Mar 8, 2020

as discussed in the diri server pls make this conform to the discord spec thanks

@tjrgg

This comment has been minimized.

Copy link
Contributor Author

tjrgg commented Mar 8, 2020

Closing in favor of an option to append to the userAgent instead.

@tjrgg tjrgg closed this Mar 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.