Skip to content

Conversation

@goncalossilva
Copy link
Member

I made some changes and #163 would've had to be rebased, so I've done that here.

@goncalossilva goncalossilva requested a review from lefcha March 31, 2025 00:38
@goncalossilva goncalossilva requested a review from a team as a code owner March 31, 2025 00:38
@goncalossilva goncalossilva requested a review from Copilot March 31, 2025 00:38
Copy link

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 integrates dataclasses_json for object mapping across various models, simplifying serialization code by removing custom from_dict/to_dict methods.

  • Updated model classes to inherit from DataClassJsonMixin
  • Removed custom serialization methods in favor of built-in functionality
  • Updated tests, dependency definitions, and changelog accordingly

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

File Description
todoist_api_python/models.py Replaces manual dict conversion with dataclasses_json mixin
tests/data/test_defaults.py Updates URL default value for consistency
pyproject.toml Adds dependency for dataclasses_json
CHANGELOG.md Documents changes to serialization and fixes

@goncalossilva
Copy link
Member Author

I'll merge this once CI is green to continue iterating based on it, but do please take a look. I'm happy to iterate after the fact.

@goncalossilva goncalossilva merged commit bd5e67d into main Mar 31, 2025
1 check passed
@goncalossilva goncalossilva deleted the dataclasses-json branch March 31, 2025 00:39
Copy link
Collaborator

@lefcha lefcha left a comment

Choose a reason for hiding this comment

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

Great, looks good!

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.

4 participants