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

Refactor and expose Item model type #28

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

david-perez
Copy link
Contributor

This commit heavily refactors the Item model type by including many (if
not all) fields returned by the Pocket API when hitting the /v3/get
endpoint 0.

The model includes all documented fields as well as other fields that
I've encountered in my experiments. Those fields that are not always
present have been typed as Options.

The commit also exposes fields publically so that consuming applications
can directly use them.

This commit heavily refactors the Item model type by including many (if
not all) fields returned by the Pocket API when hitting the /v3/get
endpoint [0].

The model includes all documented fields as well as other fields that
I've encountered in my experiments. Those fields that are not always
present have been typed as `Option`s.

The commit also exposes fields publically so that consuming applications
can directly use them.

[0]: https://getpocket.com/developer/docs/v3/retrieve
@david-perez
Copy link
Contributor Author

I've manually tested all binaries and checked they work. Of course, the file cache created by pickpocket-download now is different since it contains all these fields. So that's a breaking change to watch out for, you need to redownload and recreate it.

It proved quite annoying to have everything implement Serialize, taking into account that the composition of Serialize and Deserialize has to be the identity, and Pocket's API has some bad design choices I've not carried over to this work (e.g. storing number types as strings). I think the library should not implement Serialize and let consumer applications pull the bits they're interested in and serialize those or have them wrap Item and implement Serialize or ToSql.

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