Skip to content
This repository was archived by the owner on Apr 17, 2021. It is now read-only.

Add support for covers from URLs. Add support for PNG covers.#58

Merged
floscha merged 3 commits into
floscha:masterfrom
marccarre:issues/54-covers-from-url
Mar 19, 2019
Merged

Add support for covers from URLs. Add support for PNG covers.#58
floscha merged 3 commits into
floscha:masterfrom
marccarre:issues/54-covers-from-url

Conversation

@marccarre

@marccarre marccarre commented Mar 17, 2019

Copy link
Copy Markdown
Contributor

Fixes #54.

Changelog:

  • add support for covers from URLs (previously: only local files),
  • add support for PNG covers (previously: only JPEG),
  • segregate responsibilities between cover (only used to trigger cover upload), and imageUrl & coverImageUrl (only used to reflect Tinycards' view of the world).

N.B.: Currently rebased on top of issues/50-quiz-and-tts-options, itself rebased on top of issues/55-more-user-id-cleanup for tests to pass. Will rebase on top of master once #56 and #57 are merged.

@marccarre marccarre force-pushed the issues/54-covers-from-url branch from bc4d8c1 to 408df3d Compare March 17, 2019 09:23
… read-only purposes

This way,

- `Deck.image_url` can strictly map to Tinycards API's `imageUrl`,
- `Deck.cover_image_url` can strictly map to Tinycards API's `coverImageUrl`,
- `Deck.cover` is only used for cover upload purposes,

which is hopefully clearer, or at least, matches more closely Tinycards API's behaviour.
@marccarre marccarre force-pushed the issues/54-covers-from-url branch from 408df3d to e0c1bcb Compare March 17, 2019 20:11
@marccarre

Copy link
Copy Markdown
Contributor Author

Now rebased on top of latest master.

@coveralls

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 269

  • 51 of 55 (92.73%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.3%) to 88.847%

Changes Missing Coverage Covered Lines Changed/Added Lines %
tinycards/networking/form_utils.py 13 14 92.86%
tinycards/networking/image_utils.py 30 33 90.91%
Totals Coverage Status
Change from base Build 268: 0.3%
Covered Lines: 470
Relevant Lines: 529

💛 - Coveralls

@marccarre

Copy link
Copy Markdown
Contributor Author

@floscha, would you be fine releasing a new version of tinycards-python-api once this PR is merged? Or is there something else you'd like to have added before that?

@floscha

floscha commented Mar 19, 2019

Copy link
Copy Markdown
Owner

Hey @marccarre, I'm pretty busy with work these days but I'll have a closer look tonight and then merge the PR and release a new version if everything looks right (which so far it does) 🙂

@floscha

floscha commented Mar 19, 2019

Copy link
Copy Markdown
Owner

The code looks fine to me. Also, I've tried it out on some of my actual decks and it worked like a charm so I don't see a reason not to merge. Thanks again for the quality work!

@floscha floscha merged commit d1c9157 into floscha:master Mar 19, 2019
@floscha

floscha commented Mar 19, 2019

Copy link
Copy Markdown
Owner

As promised, I also published a new version to PyPI.

@marccarre marccarre deleted the issues/54-covers-from-url branch March 19, 2019 21:20
@marccarre

Copy link
Copy Markdown
Contributor Author

Thank you @floscha! 🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants