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

fix inc/dec operator with DecimalField + improve its doc #1872

Merged
merged 1 commit into from
Sep 4, 2018

Conversation

bagerard
Copy link
Collaborator

@bagerard bagerard commented Sep 4, 2018

Fixes inc/dec operators when using DecimalField + improve the doc and emphasize important problems of the DecimalField as it is implemented (#637)

Fixes #1517
Fixes #1320

@erdenezul
Copy link
Collaborator

@bagerard This is very educational PR. But, you ci fails with style error.

@bagerard
Copy link
Collaborator Author

bagerard commented Sep 4, 2018

argh, let me fix that

@erdenezul
Copy link
Collaborator

roar me when you fix :) @bagerard

@bagerard
Copy link
Collaborator Author

bagerard commented Sep 4, 2018

In fact it complains about

./mongoengine/queryset/transform.py:199:1: C901 'update' is too complex (48)

I've only added 2 lines there... Sounds like that method was forced through the CI at some point (or the check wasn't there back in the days). I can have a look to refactor that method but its a central piece of code in the application so its quite risky. I don't think its a good solution to do this as part of this PR

@erdenezul
Copy link
Collaborator

try to rebase master into your branch. I merged the fix

@bagerard bagerard force-pushed the inc_operator_with_decimal branch from 233cf30 to ab08e67 Compare September 4, 2018 12:56
@bagerard
Copy link
Collaborator Author

bagerard commented Sep 4, 2018

Let's see now

@erdenezul
Copy link
Collaborator

it's green

@erdenezul erdenezul merged commit d6d19c4 into MongoEngine:master Sep 4, 2018
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.

3 participants