Issue documentation improvements#201
Issue documentation improvements#201emiliano-gandini-outeda wants to merge 10 commits intobenavlabs:mainfrom
Conversation
|
I believe I'm done with this. This closes #195 |
| ): | ||
| # Check if user exists | ||
| if not await crud_users.exists(db=db, id=user_id): | ||
| if await crud_users.exists(db=db, id=user_id) is None: |
There was a problem hiding this comment.
exists returns a boolean
| db: Annotated[AsyncSession, Depends(async_get_db)] | ||
| ): | ||
| if not await crud_users.exists(db=db, id=user_id): | ||
| if await crud_users.exists(db=db, id=user_id) is None: |
There was a problem hiding this comment.
exists returns a boolean
| ): | ||
| # Check if user already exists | ||
| if await crud_users.exists(db=db, email=user_data.email): | ||
| if await crud_users.exists(db=db, email=user_data.email) is True: |
There was a problem hiding this comment.
https://peps.python.org/pep-0008/#programming-recommendations
if x is preferred to if x == True and if x is True
| ): | ||
| filters = {"is_active": is_active} | ||
| if name: | ||
| if name is True: |
There was a problem hiding this comment.
This will work only if name is a boolean, which it isn't. It's breaking logic
| ): | ||
| post = await crud_posts.get(db=db, id=post_id) | ||
| if not post: | ||
| if post is None: |
There was a problem hiding this comment.
I'd go with the other one because if we change it in the future to return something else, None will not work. Maybe some people would like to comment on this
There was a problem hiding this comment.
I think not should be kept in case falsy values are expected in the future.
However, a warning notice should be added to the website (in exceptions.md) to clearly inform users about this behavior, specifically, to make them aware of possible future changes in fast-crud regarding this.
|
@igorbenav I believe I fixed all instances of those errors |
|
Hey @emiliano-gandini-outeda, I just reviewed this PR and I'll have to close it since the boolean comparisons don't fit the standard of the code. Thanks for the contribution |
4 participants
Fixing point 8 if this Issue
is Falseinstead ofnotoperators for improved code clarityExtra:
Please review to check if proper values (True/False/None) have been used.