Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Allow marshmallow version greater than 3 #24

Merged
merged 1 commit into from
Aug 30, 2018

Conversation

rambobinator
Copy link
Contributor

No description provided.

@coveralls
Copy link

coveralls commented Aug 27, 2018

Pull Request Test Coverage Report for Build 74

  • 6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 59: 0.0%
Covered Lines: 363
Relevant Lines: 363

💛 - Coveralls

Copy link
Collaborator

@ramnes ramnes left a comment

Choose a reason for hiding this comment

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

Definitely want this in, just without having everything redefined twice. Please just put the difference between the two versions outside the function. 👍

@ramnes ramnes requested a review from trubesv August 27, 2018 14:15
@rambobinator
Copy link
Contributor Author

@trubesv What do you mean ?
It will then raise a marshmallow validation exception, which is catched by this decorator.

@rambobinator
Copy link
Contributor Author

rambobinator commented Aug 29, 2018

That's right, @ramnes maybe we should stick to marshmallow version to <3 in requirements ?

@rambobinator
Copy link
Contributor Author

There is a compatible release operator that may be used here:
marshmallow ~= 2.15.4
Is it ok for you ?

@ramnes
Copy link
Collaborator

ramnes commented Aug 29, 2018

As of now, nothing is done when there are extra fields in the payload so the requests go through, but if you update to our next release containing the bump to Marshmallow >3, you will get 400s in those cases :)

This is a non-issue because Flask-Stupe does not have any install requirement else than Flask. See setup.py (requirements.txt is just used for development). What this mean is that even if you update to our next release, that won't update Marshmallow by itself, and thus this MR is not a breaking change. It just adds support for another Marshmallow version.

@numberly numberly deleted a comment from coveralls Aug 30, 2018
@rambobinator rambobinator merged commit 5911091 into master Aug 30, 2018
@rambobinator rambobinator deleted the feature/marshmallow3 branch August 30, 2018 14:52
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.

None yet

3 participants