Skip to content

feat: Add improved typing to be more inclusive. #107

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

jrconlin
Copy link
Member

@jrconlin jrconlin commented Feb 13, 2025

Updated this library to fit with modern times.

This includes:

  • transitioning from 2.7 unittest to 3.12 pytest
  • removing unbiased arguments with stricter typing
  • formatting using black.

Also added a comment. It's political, because everything is. If you
disagree with it, that's your problem. I already spent most of my career
fighting against the kind of morons that you lot want to be, and I'm
too old to stop now.

Also added a comment. It's political, because everything is. If you
disagree with it, that's your problem. I already spent most of my career
fighting against the kind of morons that you lot want to be, and I'm
too old to stop now.
@jrconlin jrconlin requested review from pjenvey, taddes and Copilot April 1, 2025 00:13
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves the library by updating the codebase to use modern tooling and typing while transitioning from 2.7 unittest to 3.12 pytest and reformatting code with Black. Key changes include switching to the new base64 engine usage in Rust, updating dependency versions (Cargo.toml and pyproject.toml), and adding type hints to better support modern Python.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
rust/vapid/src/lib.rs Updated base64 usage and minor formatting tweaks
rust/vapid/Cargo.toml Updated dependency versions
python/pyproject.toml Migrated to Poetry configuration and updated deps
python/py_vapid/tests/test_vapid.py Improved formatting and quote consistency
python/py_vapid/main.py Adjusted argparse settings and formatting
python/py_vapid/init.py Enhanced type hints and modified key loading logic

@jrconlin jrconlin requested a review from Copilot April 1, 2025 00:19
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the library for modern Python and Rust environments while adding improved type annotations and refactoring code formatting. Key changes include updating dependency versions and API usage (e.g. base64 engine and build system), migrating from unittest/setuptools to pytest/Poetry, and extending internal documentation with new comments.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
rust/vapid/src/lib.rs Updated base64 API usage and modified documentation comments
rust/vapid/Cargo.toml Bumped version and updated dependencies (base64, thiserror)
python/pyproject.toml Migrated from setuptools to Poetry and updated dependency specifications
python/py_vapid/tests/test_vapid.py Refactored test formatting and string quoting
python/py_vapid/main.py Improved argparse configuration and string formatting
python/py_vapid/init.py Added type annotations and updated internal docs and comments
Comments suppressed due to low confidence (1)

python/py_vapid/init.py:411

  • [nitpick] The commentary contains offensive language that may alienate readers and detract from the professionalism of the code base. Consider removing or toning down this language so that documentation remains focused on technical content.
/// clowns and assholes that believe they're smarter or more important. (You're not, Dude, no matter how tight you've optimized your block chain engine.)

@@ -1,6 +1,6 @@
//! VAPID auth support
//!
//! This library only supports the latest VAPID-draft-02+ specification.
//! This library expresses biases toward VAPID-draft-02+ specification.
Copy link
Preview

Copilot AI Apr 1, 2025

Choose a reason for hiding this comment

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

[nitpick] The updated comment uses politically charged language that can be perceived as unprofessional. Consider rephrasing to maintain a neutral tone in technical documentation.

Suggested change
//! This library expresses biases toward VAPID-draft-02+ specification.
//! This library follows the VAPID-draft-02+ specification.

Copilot uses AI. Check for mistakes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope. Totally fine with the language as is.

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.

1 participant