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

linkcheck: update implementation to make use of py311 features #13036

Closed

Conversation

jayaddison
Copy link
Contributor

@jayaddison jayaddison commented Oct 18, 2024

Feature or Bugfix

  • Refactoring (mostly; does introduce small behaviour changes)

Purpose

  • Use StrEnum values to provide type-checkable linkcheck status codes.

Detail

Relates

@jayaddison jayaddison changed the title linkcheck builder: refactor status codes into an enum (take two) linkcheck builder: update implementation to make use of py311 features Oct 18, 2024
@jayaddison jayaddison changed the title linkcheck builder: update implementation to make use of py311 features linkcheck: update implementation to make use of py311 features Oct 18, 2024
@AA-Turner
Copy link
Member

Could you elaborate on the case for using StrEnum over plain strings? Can we use match/case with literal values?

I'd prefer to split the changes to use match-case and to use an enum, as they can be done separately, and it reduces the diff.

A

@jayaddison
Copy link
Contributor Author

jayaddison commented Oct 18, 2024

Could you elaborate on the case for using StrEnum over plain strings?

The main benefit I'm seeking with that change is to reduce the chance that a typo/change to a status code in one part of the code isn't caught by typechecking / code review -- currently they are string literals, so the reader has to be aware that they are used for equality checks and must be kept consistent/equal.

Can we use match/case with literal values?

Yep, that would be possible. I would personally prefer to apply the enum refactoring first, because it introduces the kind of symbol-strictness that could catch problems in a refactor that involves string literals.

I'd prefer to split the changes to use match-case and to use an enum, as they can be done separately, and it reduces the diff.

Makes sense; I think I've been a bit hasty putting this pull request together. I'll begin afresh with a simpler, enum-only refactor soon (within the next week or so) if that sounds reasonable. I'd like to get that in place before an implementation of #12985.

Edit: s/seeing/seeking -- typo with change-of-interpretation (!)

@jayaddison jayaddison closed this Oct 18, 2024
@jayaddison jayaddison deleted the refactor/linkcheck-status-codes branch October 18, 2024 18:56
@AA-Turner
Copy link
Member

That seems reasonable. In the interim we could tighten up the static typing from str to a Literal which would achieve many of the same goals, I think.

A

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