-
Notifications
You must be signed in to change notification settings - Fork 11
Add diffs key to FileChange class #290
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: master
Are you sure you want to change the base?
Conversation
…ored in mergin.json
Pull Request Test Coverage Report for Build 21880134649Details
💛 - Coveralls |
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.
Pull request overview
This PR updates the local change model to accept a diffs key (a list of diff filenames) when constructing FileChange instances from dictionaries, matching metadata produced during pull/compare flows.
Changes:
- Add a
diffsattribute toFileChangeinmergin/local_changes.py. - Update
test_local_changes_from_dictto include adiffskey in sample input data.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
mergin/local_changes.py |
Extends FileChange dataclass to include optional diffs list. |
mergin/test/test_local_changes.py |
Updates test fixture data to include diffs key when building FileChange from dicts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # some functions (MerginProject.compare_file_sets) are adding location dict to the change from project info | ||
| location: Optional[str] = None | ||
| # list of diff filenames associated with this change | ||
| diffs: Optional[List[str]] = None |
Copilot
AI
Feb 10, 2026
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.
The new diffs field defaults to None, but its doc comment describes it as a list of diff filenames. To avoid forcing callers to guard against None (and to align with how chunks is modeled), consider using field(default_factory=list) and a non-optional List[str] (or normalize None to [] in __post_init__).
| diffs: Optional[List[str]] = None | |
| diffs: List[str] = field(default_factory=list) |
| "diffs": ["diff"], | ||
| } |
Copilot
AI
Feb 10, 2026
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.
This test adds the diffs key to the input dict, but it never asserts that FileChange actually stores it. Adding an assertion like local_changes.removed[0].diffs == ["diff"] would ensure the new field is covered (and avoid the test passing if the key is silently dropped elsewhere).
from compare file_set is coming diffs key as diffs are accidentaly diffs key.