-
Notifications
You must be signed in to change notification settings - Fork 2.4k
refactor: make sure that marker_env
is complete for each type of Env
#10414
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
base: main
Are you sure you want to change the base?
Conversation
Reviewer's GuideIntroduces a formal MarkerEnv TypedDict and migrates all marker_env APIs to use it (including adding sysconfig_platform), refactors MockEnv for consistent version_info handling and override merging, updates tests to leverage the new PythonVersion type and adds an equality check across env types, and adjusts puzzle modules to accept generic Mappings and cast them for marker_env. Entity Relationship Diagram: New MarkerEnv Data StructureerDiagram
MarkerEnv {
string implementation_name
string implementation_version
string os_name
string platform_machine
string platform_release
string platform_system
string platform_version
string python_full_version
string platform_python_implementation
string python_version
string sys_platform
string version_info "PythonVersion (tuple[int,int,int,str,int])"
string interpreter_name
string interpreter_version
string sysconfig_platform "New attribute"
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @radoering - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
1a17ed1
to
feeee27
Compare
cbbf051
to
cae264b
Compare
* add a test that checks that `marker_env` is equal for all types of envs * make `marker_env` a `TypedDict` * add `sysconfig_platform` to `SystemEnv.marker_env` (is not used so that it is not a relevant bug that it was missing)
cae264b
to
253a30a
Compare
marker_env
is equal for all types of envsmarker_env
aTypedDict
sysconfig_platform
toSystemEnv.marker_env
(is not used so that it is not a relevant bug that it was missing)Pull Request Check List
Resolves: #issue-number-here
Summary by Sourcery
Define a TypedDict for environment markers, enforce complete marker_env structures across all Env implementations, and add a test to ensure consistency
Bug Fixes:
Enhancements:
Tests:
Chores: