Skip to content

Dog status codes #571

Merged
Xithrius merged 7 commits into
python-discord:masterfrom
Shivansh-007:feature/http-dog
Feb 4, 2021
Merged

Dog status codes #571
Xithrius merged 7 commits into
python-discord:masterfrom
Shivansh-007:feature/http-dog

Conversation

@Shivansh-007
Copy link
Copy Markdown
Contributor

@Shivansh-007 Shivansh-007 commented Jan 30, 2021 •
edited
Loading

Relevant Issues

Closes #568

Description

HTTP status dogs doesn't the following codes: [101, 102, 421, 511, 499, 599, 415].
I have made a group for cat and dog status codes and dog status code is just duplicate to that of the cat just it adds another check by checking the response status after requesting the url.

Screenshots

Screenshot from 2021-01-31 05-19-21
Screenshot from 2021-01-31 05-19-39

Did you:

@Shivansh-007 Shivansh-007 changed the title Add http status dog and add http_cat to group http_status Dog status codes Jan 30, 2021
@Xithrius Xithrius added area: backend Related to internal functionality and utilities level: 0 - beginner status: needs review Author is waiting for someone to review and approve type: feature Relating to the functionality of the application. labels Jan 31, 2021
ChrisLovering
ChrisLovering requested changes Jan 31, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@ChrisLovering ChrisLovering left a comment

Choose a reason for hiding this comment

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

I think we should rename the cog. The two could do's are optional, if you feel like implementing them. If not, they can be raised as separate issues :)

Comment thread bot/exts/evergreen/status_codes.py Outdated
Comment thread bot/exts/evergreen/status_codes.py Outdated
Comment thread bot/exts/evergreen/status_codes.py Outdated
@Shivansh-007
rename the cog to HTTPStatusCodes
c87f6e0
@Shivansh-007
Add similar repsonse.status check to too, and instead of ValueError r…
e32182e 
…aise NotImplemented error if the status is not implemented yet but is a valid status code
ChrisLovering
ChrisLovering requested changes Feb 1, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@ChrisLovering ChrisLovering left a comment

Choose a reason for hiding this comment

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

Just 1 change outstanding. I've tested the rest of this locally and it seems good to me! Nice job :)

Comment thread bot/exts/evergreen/status_codes.py
@Xithrius
Copy link
Copy Markdown
Contributor

Xithrius commented Feb 1, 2021

When you input 101 for the dog status codes, what does it output? A 404?

@ChrisLovering
Copy link
Copy Markdown
Member

ChrisLovering commented Feb 1, 2021

When you input 101 for the dog status codes, what does it output? A 404?

image

@Xithrius
Copy link
Copy Markdown
Contributor

Xithrius commented Feb 1, 2021

Sorry, what I mean is when requesting from the url for a 101, what does it return?

@ChrisLovering
Copy link
Copy Markdown
Member

ChrisLovering commented Feb 1, 2021

Sorry, what I mean is when requesting from the url for a 101, what does it return?

My bad, any code that hasn't been implemented redirects to https://httpstatusdogs.com/404-not-found which is a page with a 404 image, but a bunch of other content. This is the case for 101, but also 999 as example.

@Xithrius
Copy link
Copy Markdown
Contributor

Xithrius commented Feb 1, 2021

I see, thank you for this information.

@Shivansh-007
Copy link
Copy Markdown
Contributor Author

Shivansh-007 commented Feb 1, 2021

@ChrisLovering Mentioned the requested changes!

ChrisLovering
ChrisLovering approved these changes Feb 1, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@ChrisLovering ChrisLovering left a comment

Choose a reason for hiding this comment

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

LGTM!

Xithrius
Xithrius requested changes Feb 3, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@Xithrius Xithrius left a comment

Choose a reason for hiding this comment

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

Looks pretty darn good to me, besides a few changes.

Since the help command shows up as the screenshot below, I thought it would be better if the command was named directly after the animal.

image

Comment thread bot/exts/evergreen/status_codes.py Outdated
Comment thread bot/exts/evergreen/status_codes.py Outdated
Comment thread bot/exts/evergreen/status_codes.py Outdated
Comment thread bot/exts/evergreen/status_codes.py
Comment thread bot/exts/evergreen/status_codes.py Outdated
@Shivansh-007
Copy link
Copy Markdown
Contributor Author

Shivansh-007 commented Feb 3, 2021

Mentioned the requested changes @Xithrius !

@Xithrius Xithrius merged commit 35038ac into python-discord:master Feb 4, 2021
@Shivansh-007 Shivansh-007 deleted the feature/http-dog branch February 7, 2021 11:12
@Xithrius Xithrius removed the status: needs review Author is waiting for someone to review and approve label Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ChrisLovering ChrisLovering ChrisLovering approved these changes

@Xithrius Xithrius Xithrius approved these changes

Assignees

No one assigned

Labels

area: backend Related to internal functionality and utilities type: feature Relating to the functionality of the application.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Dog status codes

3 participants

@Shivansh-007 @Xithrius @ChrisLovering