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

chore: typing cleanup #2339

Merged
merged 16 commits into from
Sep 27, 2024
Merged

chore: typing cleanup #2339

merged 16 commits into from
Sep 27, 2024

Conversation

CaselIT
Copy link
Member

@CaselIT CaselIT commented Sep 25, 2024

Fixes: #2313

Cannot really came up with many type aliases that may be nice to have public.
For not I've found only 3. https://github.com/falconry/falcon/pull/2339/files#diff-59734eebd58dfd39eafe7094e8725d6d5b3847156d7d0bff3cbdf5095fcf84e4

Also have yet to look at naming.

@CaselIT CaselIT requested a review from vytas7 September 25, 2024 20:15
Copy link

codecov bot commented Sep 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (9b0f7da) to head (4efdb04).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #2339   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           63        64    +1     
  Lines         7553      7621   +68     
  Branches      1239      1242    +3     
=========================================
+ Hits          7553      7621   +68     

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

@CaselIT
Copy link
Member Author

CaselIT commented Sep 25, 2024

Not sure if we could provide a Request subclass there that's generic on the media?
So people can type their method as def on_get(self, req: Request[list[int]], resp: Response)

@vytas7 vytas7 changed the title Typing cleanup chore: typing cleanup Sep 25, 2024
@CaselIT CaselIT marked this pull request as ready for review September 26, 2024 21:55
Copy link
Member

@vytas7 vytas7 left a comment

Choose a reason for hiding this comment

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

There is an issue with coverage.

I'll try to skim through other changes today too.

falcon/_typing.py Show resolved Hide resolved
@CaselIT
Copy link
Member Author

CaselIT commented Sep 27, 2024

There is an issue with coverage.

I believe it's its usual issues, but I haven't checked it

Copy link
Member

@vytas7 vytas7 left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the whole typing effort, this is almost ready to rock.

We just need to address missing coverage, and make a decision on UNSET before merging.

We could probably bikeshed some names too, but since they are private under _typing, we can revise them later at any time.

falcon/media/multipart.py Outdated Show resolved Hide resolved
falcon/media/multipart.py Outdated Show resolved Hide resolved
falcon/testing/srmock.py Outdated Show resolved Hide resolved
@CaselIT CaselIT requested a review from vytas7 September 27, 2024 18:36
Copy link
Member

@vytas7 vytas7 left a comment

Choose a reason for hiding this comment

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

Looks great now 💯
(Potentially bar naming refinement, but since these are strictly private under _typing, we can improve them at any time.)

@vytas7 vytas7 merged commit 0c24a18 into falconry:master Sep 27, 2024
37 checks passed
@CaselIT CaselIT deleted the typing_cleanup branch September 29, 2024 08:41
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.

Typing cleanup for 4.0
2 participants