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

Add 497: HTTP Request sent to HTTPS port #124

Merged
merged 10 commits into from
Jul 28, 2021
Merged

Conversation

zurgeg
Copy link
Contributor

@zurgeg zurgeg commented Jul 27, 2021

No description provided.

@zurgeg zurgeg mentioned this pull request Jul 27, 2021
17 tasks
@rogeriopvl
Copy link
Member

Hi @zurgeg , thanks for the contribution. There are a couple of issues with the PR that we need to clear out before merging:

  • The image is heavy (almost 1Mb, if you check most of our images are around 40Kb), please run an optimizer before adding it
  • You used the portrait template, but the image seems more suited for the landscape template (please check the template instructions on the README file)
  • You need to add the status code in the src/lib/statuses.js file, otherwise it won't show up in the website

Cheers!

@zurgeg
Copy link
Contributor Author

zurgeg commented Jul 27, 2021

Hi @zurgeg , thanks for the contribution. There are a couple of issues with the PR that we need to clear out before merging:

* The image is heavy (almost 1Mb, if you check most of our images are around 40Kb), please run an optimizer before adding it

* You used the portrait template, but the image seems more suited for the landscape template (please check the template instructions on the README file)

* You need to add the status code in the `src/lib/statuses.js` file, otherwise it won't show up in the website

Cheers!

Thanks for the advice! I didn't really read the README much (sorry for that 😅), but I actually used esmBot to create the image, I will recreate this taking that in mind!

@zurgeg
Copy link
Contributor Author

zurgeg commented Jul 27, 2021

@rogeriopvl I fixed the issues you told me about, how's it looks now?

src/lib/statuses.js Outdated Show resolved Hide resolved
@rogeriopvl rogeriopvl changed the title Add 497: HTTP Request sent to HTTP port Add 497: HTTP Request sent to HTTPS port Jul 27, 2021
Co-authored-by: Rogério Vicente <rogerio@hey.com>
@zurgeg
Copy link
Contributor Author

zurgeg commented Jul 27, 2021

@rogeriopvl I fixed the issue you mentioned, how's it now?

src/lib/statuses.js Outdated Show resolved Hide resolved
zurgeg and others added 2 commits July 27, 2021 17:07
Co-authored-by: Rogério Vicente <rogerio@hey.com>
@zurgeg
Copy link
Contributor Author

zurgeg commented Jul 27, 2021

@rogeriopvl Alright, should be everything right?

@rogeriopvl
Copy link
Member

😅 the image has the word "port" in lowercase

@zurgeg
Copy link
Contributor Author

zurgeg commented Jul 28, 2021

😅 the image has the word "port" in lowercase

Loool, oops

@zurgeg
Copy link
Contributor Author

zurgeg commented Jul 28, 2021

Alright, @rogeriopvl I fixed that

@rogeriopvl
Copy link
Member

Great @zurgeg , I'm going to merge this! Thanks! 👍

@rogeriopvl rogeriopvl merged commit ed02e4f into httpcats:master Jul 28, 2021
@rogeriopvl
Copy link
Member

@all-contributors add @zurgeg for code

Copy link
Contributor

@rogeriopvl

I've put up a pull request to add @zurgeg! 🎉

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.

2 participants