Skip to content

Conversation

@mahdirahimi1999
Copy link

Description:

This pull request improves the test suite by replacing raw status codes with httpx.codes constants for better readability and consistency with the httpx package conventions.

Changes:

  • Replaced raw status code 200 with httpx.codes.OK.value in the tests.
  • Replaced raw status code 500 with httpx.codes.INTERNAL_SERVER_ERROR.value in the tests.

Rationale:

  • The httpx.codes module provides human-readable status codes, making the tests easier to understand and maintain.
  • Replacing raw status codes (e.g., 200, 500) with the corresponding httpx.codes constants improves consistency with the httpx library's intended usage and avoids the use of "magic numbers" in the test code.

Why this PR is Important:

  • Improved Readability: Makes the test code more intuitive, especially for contributors unfamiliar with HTTP status codes.
  • Better Consistency: Aligns the test code with the httpx package's built-in status code handling.
  • Maintainability: Helps prevent errors related to hardcoded status codes by using standardized constants.

Request for Review:

Please review the changes and let me know if any further modifications or refinements are needed. This small enhancement will make the tests more consistent and easier to understand for all contributors.

Updated the test to use httpx.codes.OK directly instead of httpx.codes.OK.value for better readability and consistency with httpx package conventions.
@mahdirahimi1999
Copy link
Author

Hi @tomchristie, could you please review these changes?

@Kludex
Copy link
Contributor

Kludex commented Dec 3, 2024

Thanks for the PR @mahdirahimi1999 , but I think this is opinionated, and it doesn't bring much improvement on readability.

Also, PRs that are only refactor, that are a non-clear win, would probably have the same reaction from other maintainers.

@Kludex Kludex closed this Dec 3, 2024
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