-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
WIP influx packages #257
WIP influx packages #257
Conversation
fc2c576
to
31404a8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that aint it
Woops didn't mean to click approve because there are still merge conflicts
Other than that it needs manual testing this looks ready-ish? @kyranet |
@swiizyy when will you pick up the remaining TODOs? |
} | ||
|
||
export function initializeInflux(options: Client.Options = {}) { | ||
if (!process.env.INFLUX_URL) return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also check whether INFLUX_ENABLED
is true (envParseBoolean('INFLUX_ENABLED', true)
)
|
||
protected initTags() { | ||
this.tags.push( | ||
[Tags.Client, envParseString('CLIENT_ID', Buffer.from(envParseString('DISCORD_TOKEN').split('.')[0], 'base64').toString())], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This always processes the fallback, it's better to do process.env.CLIENT_ID ?? Buffer.from(...).toString()
.
But at that point, does http-framework
not provide a function for getting the client ID? o.o
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it doesn't seem to me, that's why I reused this... Do you want me to add this option to http-framework
?
packages/shared-influx-pieces/src/lib/utils/ApproximateGuilds.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This entire README needs a rewrite, most of it is shared-http-pieces
.
@@ -1,3 +1,3 @@ | |||
import { createTsupConfig } from '../../scripts/tsup.config'; | |||
|
|||
export default createTsupConfig({ format: ['esm', 'cjs'] }); | |||
export default createTsupConfig({ format: ['esm'] }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we doing this? cc @favna
Todos