Skip to content

Code clean-up for core user module#591

Merged
phorward merged 6 commits intoviur-framework:developfrom
phorward:refactor-user
Dec 20, 2022
Merged

Code clean-up for core user module#591
phorward merged 6 commits intoviur-framework:developfrom
phorward:refactor-user

Conversation

@phorward
Copy link
Copy Markdown
Member

@phorward phorward commented Dec 12, 2022

  • Cleaned imports
  • Cleaned obsolete notations (super-calls, inherit from object)
  • Renamed classes to be PEP-8 compliant
  • Cleaned code-style
  • Deprecation warning for userSkel use, rather than UserSkel

@phorward
Code clean-up for core user module
904f5f4 
- Cleaned imports
- Cleaned obsolete notations (super-calls, inherit from object)
- Renamed classes to be PEP-8 compliant
- Cleaned code-style
- Deprecation warning for `userSkel` use, rather than `UserSkel`
ArneGudermann
ArneGudermann reviewed Dec 14, 2022
View reviewed changes
Comment thread core/modules/user.py Outdated
@phorward phorward mentioned this pull request Dec 14, 2022
Closed
phorward and others added 2 commits December 14, 2022 12:54
@phorward @ArneGudermann
Apply suggestions from code review
aed801a 
Co-authored-by: agudermann <47318461+ArneGudermann@users.noreply.github.com>
@phorward
Fixing incomplete variable renaming
8fa1eda
@ArneGudermann
Copy link
Copy Markdown
Contributor

ArneGudermann commented Dec 14, 2022

Hello @phorward. Now I'm fine with this but I have one last suggestion can you please change this
skel.password = PasswordBone(descr="Passwort", required=False)
to:
skel.password.required = False
here :

viur-core/core/modules/user.py

Line 686 in 204f520

skel.password = PasswordBone(descr="Passwort", required=False)

skel.password is already and PasswordBone in this case.

@phorward
Copy link
Copy Markdown
Member Author

phorward commented Dec 14, 2022

Hello @phorward. Now I'm fine with this but I have one last suggestion can you please change this skel.password = PasswordBone(descr="Passwort", required=False) to: skel.password.required = False here :

Wilco. I'll use this pull request to integrate viur-framework/server#24 (comment) as well, IMHO this was a senseless discussion.

@phorward
Copy link
Copy Markdown
Member Author

phorward commented Dec 14, 2022

@ArneGudermann please re-check the latest changes. Afterwards we can approve it.

@phorward phorward merged commit f4b829f into viur-framework:develop Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ArneGudermann ArneGudermann ArneGudermann approved these changes

@sveneberth sveneberth Awaiting requested review from sveneberth

@akelch akelch Awaiting requested review from akelch

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@phorward @ArneGudermann