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

Added custom user model with customer user manager #6

Merged
merged 9 commits into from
May 13, 2024

Conversation

bartczak-pa
Copy link
Contributor

This overwrites default django behaviour and allows to create users based on email instead of username.

Can we check if this code is okay?

This overwrites default django behaviour and allows to create users based on email instead of username
@bartczak-pa bartczak-pa requested a review from a team May 11, 2024 16:54
@jacoor
Copy link
Contributor

jacoor commented May 11, 2024

@bartczak-pa please see tests results. Those failed.
https://github.com/pymasterspl/reddit/actions/runs/9045129435/job/24854541143
If you look here, it failed on migrations. This will be checked only after all is green. Please fix migrations.

This overwrites default django behaviour and allows to create users based on email instead of username
This overwrites default django behaviour and allows to create users based on email instead of username
@bartczak-pa
Copy link
Contributor Author

@jacoor Migrations are fixed and code is passing tests now.

Copy link
Contributor

@jacoor jacoor left a comment

Choose a reason for hiding this comment

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

The code looks good.
However, Ruff is configured on purpose to enforce typings.
Please, do fix ruff errors and avoid #noqa unless really necessary.
I pushed an example of type updates for you, please fix other missing types.
If you have issues with typing, you can try to use AI to fix

  • copilot - ask it to add typings for you (this works)
  • or copy the code to Chat GPT and ask to add types.
    Of course, do not forget to use ruff check --fix for other issues.
    Again, only use #noqa if this is really necessary, like:
    users/models.py:29:19: TRY003 Avoid specifying long messages outside the exception class, however, for this one google has very good answer you can find after couple seconds search:
    https://docs.astral.sh/ruff/rules/raise-vanilla-args/
    (I put "how to fix TRY003 Avoid specifying long messages outside the exception class`" as a question to google).

This overwrites default django behaviour and allows to create users based on email instead of username
@jacoor
Copy link
Contributor

jacoor commented May 13, 2024

@bartczak-pa you need to resolve conflicts. After that, tests will start to run and we will know if the code good.
Let me know if you need help.

@jacoor
Copy link
Contributor

jacoor commented May 13, 2024

Great work. Now merge away!

@bartczak-pa
Copy link
Contributor Author

Typings and conflicts are resolved.

@bartczak-pa bartczak-pa merged commit 13fc39f into dev May 13, 2024
2 checks passed
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.

2 participants