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

Call super().clean instead of accessing self.cleaned_data directly #717

Merged
merged 1 commit into from Nov 16, 2017

Conversation

atombrella
Copy link
Contributor

Not sure what the motivation for not calling clean, but you're missing out on all of django's built-in validation.

@codecov
Copy link

codecov bot commented Nov 16, 2017

Codecov Report

Merging #717 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #717      +/-   ##
==========================================
+ Coverage   69.95%   69.97%   +0.02%     
==========================================
  Files          89       89              
  Lines        4423     4426       +3     
==========================================
+ Hits         3094     3097       +3     
  Misses       1329     1329
Impacted Files Coverage Δ
src/wiki/forms.py 63.71% <100%> (+0.2%) ⬆️
src/wiki/plugins/attachments/forms.py 63.71% <100%> (+0.32%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd04b5e...e0f0bc5. Read the comment docs.

@benjaoming
Copy link
Member

Oh woops, bad mistake, could cause issues.

@benjaoming
Copy link
Member

Seeing 100% diff coverage and that tests are fine would indicate that none of these were intentional in leaving out super()

@benjaoming benjaoming merged commit ee6a8ac into django-wiki:master Nov 16, 2017
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.

None yet

2 participants