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

chore: enable flake8-django (DJ) rule in ruff config #779

Merged
merged 1 commit into from
Nov 29, 2023

Conversation

afuetterer
Copy link
Member

@afuetterer afuetterer commented Oct 16, 2023

Description

This PR is an attempt to enable the flake8-django (DJ) rule in the ruff config.

The first and easiest fix was to reorder method and manager declarations to have the order the ruleset expects them

What do you think about these rules?

To me both rules make sense, but we could also ignore them.

DJ007 would mean to explicitly name all fields that are exposed through the modelform, which might be a good idea anyway in case not all of them should be exposed?

DJ001 would mean to alter the model fields (add default="") to TextFields and generate new migrations. To me the rule makes sense: not to have an empty string and null possible for TextFields.

But what do you think?

rdmo/conditions/admin.py:13:9: DJ007 Do not use `__all__` with `ModelForm`, use `fields` instead
rdmo/questions/models/question.py:26:11: DJ001 Avoid using `null=True` on string-based fields such as URLField
rdmo/questions/models/question.py:36:16: DJ001 Avoid using `null=True` on string-based fields such as CharField
rdmo/questions/models/question.py:41:15: DJ001 Avoid using `null=True` on string-based fields such as TextField
rdmo/questions/models/question.py:71:18: DJ001 Avoid using `null=True` on string-based fields such as TextField
rdmo/questions/models/question.py:76:18: DJ001 Avoid using `null=True` on string-based fields such as TextField
rdmo/questions/models/question.py:81:18: DJ001 Avoid using `null=True` on string-based fields such as TextField
rdmo/questions/models/question.py:86:18: DJ001 Avoid using `null=True` on string-based fields such as TextField
rdmo/questions/models/question.py:91:18: DJ001 Avoid using `null=True` on string-based fields such as TextField
rdmo/questions/models/question.py:96:18: DJ001 Avoid using `null=True` on string-based fields such as TextField
rdmo/questions/models/question.py:101:18: DJ001 Avoid using `null=True` on string-based fields such as TextField
rdmo/questions/models/question.py:106:18: DJ001 Avoid using `null=True` on string-based fields such as TextField
rdmo/questions/models/question.py:111:18: DJ001 Avoid using `null=True` on string-based fields such as TextField
rdmo/questions/models/question.py:116:18: DJ001 Avoid using `null=True` on string-based fields such as TextField
rdmo/questions/models/question.py:121:26: DJ001 Avoid using `null=True` on string-based fields such as TextField
rdmo/questions/models/question.py:126:26: DJ001 Avoid using `null=True` on string-based fields such as TextField
rdmo/questions/models/question.py:131:26: DJ001 Avoid using `null=True` on string-based fields such as TextField
rdmo/questions/models/question.py:136:26: DJ001 Avoid using `null=True` on string-based fields such as TextField
rdmo/questions/models/question.py:141:26: DJ001 Avoid using `null=True` on string-based fields such as TextField
rdmo/tasks/admin.py:15:9: DJ007 Do not use `__all__` with `ModelForm`, use `fields` instead
rdmo/views/admin.py:15:9: DJ007 Do not use `__all__` with `ModelForm`, use `fields` instead
Found 21 errors.

Related issue: #766

Types of Changes

  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)

Checklist

  • I have read the contributor guide.
  • My code follows the code style of this project.
  • All new and existing tests passed.

Tasks

  • resolve DJ007 violations
  • decide whether to resolve or ignore DJ001 violations -> resolve
  • resolve DJ001: replace null=True, blank=True with blank=True, default=""
  • generate new migrations
  • fix failing tests: create new fixtures
  • rebase branch
  • re-create migrations

@afuetterer afuetterer linked an issue Oct 16, 2023 that may be closed by this pull request
@afuetterer afuetterer marked this pull request as draft October 16, 2023 04:57
@jochenklar

This comment was marked as resolved.

MyPyDavid

This comment was marked as resolved.

@afuetterer

This comment was marked as resolved.

@afuetterer

This comment was marked as resolved.

@afuetterer

This comment was marked as resolved.

@jochenklar

This comment was marked as resolved.

@afuetterer

This comment was marked as resolved.

@afuetterer afuetterer self-assigned this Oct 19, 2023
@afuetterer afuetterer force-pushed the 766-flake8-django branch 2 times, most recently from e97f09a to 96bed38 Compare October 19, 2023 09:30
@afuetterer afuetterer marked this pull request as ready for review October 19, 2023 09:31
@afuetterer afuetterer requested a review from MyPyDavid October 19, 2023 09:33
@afuetterer

This comment was marked as resolved.

@afuetterer afuetterer marked this pull request as draft October 19, 2023 10:45
@afuetterer
Copy link
Member Author

Created new questions fixtures, using:

python manage.py loaddata fixtures/*
python manage.py makemigrations
python manage.py migrate
python manage.py dumpdata questions | python -m json.tool --indent 2 > questions.json

@afuetterer afuetterer marked this pull request as ready for review October 20, 2023 09:48
@afuetterer

This comment was marked as resolved.

@afuetterer

This comment was marked as resolved.

@jochenklar jochenklar added this to the 2.1.0 milestone Oct 27, 2023
@jochenklar

This comment has been minimized.

@jochenklar jochenklar added the migrations Indicates that this PR contains migrations label Nov 19, 2023
@afuetterer
Copy link
Member Author

Rebased and ready for review.

@afuetterer
Copy link
Member Author

Can we merge this @MyPyDavid?

@MyPyDavid
Copy link
Member

Yes please, I have approved already right

@afuetterer afuetterer merged commit f7c0972 into rdmorganiser:dev-2.1.0 Nov 29, 2023
12 checks passed
@afuetterer afuetterer deleted the 766-flake8-django branch November 29, 2023 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
migrations Indicates that this PR contains migrations type:maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable flake8-django (DJ) rule in ruff config
3 participants