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

refactor!: Retry argument attributes #40

Merged
merged 1 commit into from
Jul 3, 2024

Conversation

jmgate
Copy link
Collaborator

@jmgate jmgate commented Jul 3, 2024

Type: Refactor

Description

Mypy doesn't like that the retry argument attributes are defined dynamically when a class is instantiated, so it'll complain that they don't exist when a subclass developer is trying to modify them when overriding/extending the parser. If instead of having separate attributes per registered stage, we just have a `dict`` where the keys are the stage names, mypy is happy.

Note that this is a breaking change, as it changes how subclass developers customize the parser.

Related Issues/PRs

Closes #22.

@jmgate jmgate self-assigned this Jul 3, 2024
@jmgate jmgate marked this pull request as draft July 3, 2024 22:12
Copy link

codecov bot commented Jul 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (bf9017a) to head (730e6cb).
Report is 2 commits behind head on next.

Additional details and impacted files
@@            Coverage Diff            @@
##              next       #40   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines          180       186    +6     
  Branches        21        21           
=========================================
+ Hits           180       186    +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jmgate
Copy link
Collaborator Author

jmgate commented Jul 3, 2024

Still need to check the Examples page in the docs to see if all is well.

Mypy doesn't like that the retry argument attributes are defined
dynamically when a class is instantiated, so it'll complain that they
don't exist when a subclass developer is trying to modify them when
overriding/extending the parser.  If instead of having separate
attributes per registered stage, we just have a `dict`` where the keys
are the stage names, mypy is happy.

Note that this is a breaking change, as it changes how subclass
developers customize the parser.
@jmgate jmgate force-pushed the 22-refactor-retry-argument-attributes branch from dffc08c to 730e6cb Compare July 3, 2024 22:39
@jmgate
Copy link
Collaborator Author

jmgate commented Jul 3, 2024

The Examples page is good to go. I'm going to hold off on merging, though, to see if we want to batch up some breaking changes into a single PR (a single major version bump).

@jmgate jmgate changed the base branch from master to next July 3, 2024 23:07
@jmgate jmgate marked this pull request as ready for review July 3, 2024 23:07
@jmgate jmgate merged commit a06b7bf into next Jul 3, 2024
14 checks passed
@jmgate jmgate deleted the 22-refactor-retry-argument-attributes branch July 3, 2024 23:08
@jmgate
Copy link
Collaborator Author

jmgate commented Jul 3, 2024

I went ahead and merged this into the next branch, where I'll collect breaking changes for a future major release.

@jmgate jmgate mentioned this pull request Jul 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.

BREAKING: Refactor retry argument attributes
1 participant