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

Fix #1182 | Changed regexp to match standard ngrok urls #1311

Merged
merged 1 commit into from
Jan 6, 2022
Merged

Fix #1182 | Changed regexp to match standard ngrok urls #1311

merged 1 commit into from
Jan 6, 2022

Conversation

hpatoio
Copy link
Contributor

@hpatoio hpatoio commented Aug 31, 2021

What this PR does

I've changed the regexp used to whitelist requests. By default ngrok urls contains - but the regexp used at the moment only allows letters and numbers.

With this change requests to URLs like https://cee0-212-222-187-147.ngrok.io work

This issue was reported in #1182

@hpatoio
Copy link
Contributor Author

hpatoio commented Sep 24, 2021

Is anyone checking PRs for this project ?

@hpatoio
Copy link
Contributor Author

hpatoio commented Oct 25, 2021

What prevent this from been merged ? It's October so I would like to give my 2 cents to #hacktoberfest
😸

@ghost ghost added the cla-needed label Dec 22, 2021
@hpatoio
Copy link
Contributor Author

hpatoio commented Dec 22, 2021

I'm trying to sign the CLA but I get this error Submission rate limit exceeded. Please try again later

@BranLiang
Copy link
Contributor

I'm trying to sign the CLA but I get this error Submission rate limit exceeded. Please try again later

Please try again, also test is failing, please fix that as well.

@hpatoio
Copy link
Contributor Author

hpatoio commented Dec 23, 2021

Tried to sign the CLA but I still get the very same error.

I've fixed the test and rebased on latest master

@BranLiang
Copy link
Contributor

@hpatoio Hmm, that's weird. According to this ticket IBM/sarama#2006, someone had the same problem before, and it was resolved after several tries. Could you please also try again?

@hpatoio
Copy link
Contributor Author

hpatoio commented Dec 24, 2021

I tried 20something times in a 3 hours period. Same error. I've also asked a friend of mine and he got the very same message.

@BranLiang
Copy link
Contributor

@hpatoio Sorry for the inconvenience 😭, let me ask around after the holiday to see if I can find a solution for CLA.

@hpatoio
Copy link
Contributor Author

hpatoio commented Jan 5, 2022

It worked ! I was able to sign the CLA. Can you re run the pipeline ?

@ghost ghost removed the cla-needed label Jan 6, 2022
@BranLiang BranLiang merged commit a7ce777 into Shopify:master Jan 6, 2022
@NabeelAhsen NabeelAhsen mentioned this pull request Jan 7, 2022
4 tasks
@shopify-shipit shopify-shipit bot temporarily deployed to rubygems January 7, 2022 20:27 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants