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: now have 103 and 426 http-codes #89

Merged
merged 6 commits into from
Sep 20, 2023

Conversation

angellee177
Copy link
Contributor

@angellee177
Copy link
Contributor Author

@prettymuchbryce please kindly review this PR. :)

because the docs will be generated, we don't need to add it manually.
@angellee177
Copy link
Contributor Author

@prettymuchbryce is there anything i need to changes or fix?

Copy link

@pacifiquem pacifiquem left a comment

Choose a reason for hiding this comment

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

@angellee177 map this issue ( #20 ) to this pull request so that it'll be closed as soon as this pull request is merged.
also everything looks good @prettymuchbryce it can be merged.

@angelsupp
Copy link
Contributor

thank you for reviewing my PR @pacifiquem :) can you help me to merge the PR?

@pacifiquem
Copy link

No @angelsupp we've to wait for @prettymuchbryce or any other maintainer to merge it. Since everything looks good let's hope they'll merge it soon :).

@angelsupp
Copy link
Contributor

thanks a lot @pacifiquem 😆

@prettymuchbryce
Copy link
Owner

prettymuchbryce commented Sep 18, 2023

Thank you and apologies for my delayed response. It looks like HTTP 521 is not an official status code, but instead is specific to CloudFlare. The documentation linked in this PR here is for an unrelated SMTP (not HTTP) error.

I would prefer not to include codes without an official RFC. If we can remove HTTP 521 from this PR then I will happily merge the PR as 103 and 426 look good to me!

Copy link

@pacifiquem pacifiquem left a comment

Choose a reason for hiding this comment

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

@angellee177 @angelsupp remove 521 status code as suggested by @prettymuchbryce and this PR will be merged.

@pacifiquem
Copy link

Closes #20

@angelsupp
Copy link
Contributor

let me remove the 521 status code @pacifiquem @prettymuchbryce

@angelsupp
Copy link
Contributor

done @prettymuchbryce @pacifiquem 😆
here is the commit.

Copy link

@pacifiquem pacifiquem left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@pacifiquem
Copy link

@prettymuchbryce let's merge it.

@prettymuchbryce prettymuchbryce merged commit b04f738 into prettymuchbryce:master Sep 20, 2023
0 of 3 checks passed
@pacifiquem
Copy link

Thank you @angelsupp for your contribution.

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.

None yet

4 participants