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.

Sign up for GitHub

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

Jump to bottom

Add support for PTB/Canary Webhook URLs #12

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add support for PTB/Canary Webhook URLs #12

wants to merge 3 commits into from

Conversation

aw-0
Copy link

@aw-0 aw-0 commented Apr 19, 2022

Added support for Discord's Canary/PTB Webhook URLs and touched up some function documentation to fit the changes. Love LDS btw ❤️

@aw-0
🔗 Add support for PTB/Canary Webhook URLs
9fbf653
@aw-0
📃 Function Doc Fixes
396b576 
Haven't used this type of documentation before so lmk if this isn't conventional
@vercel
Copy link

vercel bot commented Apr 19, 2022 •
edited

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/alii/linear-discord-serverless/62EvRSGMVYeAF6P5VrPJYoGVzLce
✅ Preview: https://linear-discord-serverless-git-fork-aw-0-aw0-patch-1-alii.vercel.app

sasial-dev
sasial-dev reviewed Apr 19, 2022
View reviewed changes
Copy link

@sasial-dev sasial-dev left a comment

Choose a reason for hiding this comment

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

A point to consider!

public/index.html Outdated Show resolved Hide resolved
@AussieDoggo
Copy link

yes

@alii
Copy link
Owner

alii commented Apr 19, 2022

Thanks for the PR! Glad you like LDS :) Would it at all be possible if we could implement a regex as mentioned in a review comment. ❤️

@aw-0
Copy link
Author

aw-0 commented Apr 20, 2022

Added regex testing under isWebhook. I've also attempted to create regex to account for these changes in parseWebhook over the last hour or so to no avail, as I'm not very experienced in regex. Will continue trying and commit the rest.

sasial-dev
sasial-dev suggested changes Apr 20, 2022
View reviewed changes
public/index.html
Comment on lines +93 to +94
if (url.includes('ptb.')) url = url.replace('ptb.', '');
if (url.includes('canary.')) url = url.replace('canary.', '');
Copy link

Choose a reason for hiding this comment

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

You don't need this!

Suggested change
if (url.includes('ptb.')) url = url.replace('ptb.', '');
if (url.includes('canary.')) url = url.replace('canary.', '');

Copy link

Choose a reason for hiding this comment

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

Instead use a regex capture group, and strip the inital section of the URL!

Copy link
Author

Choose a reason for hiding this comment

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

That was my plan - see #12 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alii alii alii left review comments

@sasial-dev sasial-dev sasial-dev requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@aw-0 @AussieDoggo @alii @sasial-dev