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

add core app #1

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

add core app #1

wants to merge 3 commits into from

Conversation

ahmed-arb
Copy link
Owner

@ahmed-arb ahmed-arb commented Jan 11, 2023

What?

Add core app and user authentication. App include; book, loan, and author models and their migrations, views, and URLs.

Why?

These changes complete the basic library CRUD and authentication.

How

User authentication is implemented using Djoser and simplejwt.

Copy link
Collaborator

@abdullahwaheed abdullahwaheed left a comment

Choose a reason for hiding this comment

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

overall pretty good work. Here are some suggestion:

  • Please take a look at pylint. its a linter for python. in the future assignments, we will be imposing pylint to make sure our code meet pep-8 style guide for python
  • PR title should be concise and descriptive and you should add a short PR description describing what the PR is about.
  • Commit your code whenever you add a functionality. e.g. when you have setup project for the first time, that should be first commit. When you added models and migration, that should be second commit.
  • Commit message should be meaningful and should follow these conventions

core/migrations/0001_initial.py Outdated Show resolved Hide resolved
core/admin.py Outdated Show resolved Hide resolved
removed redundant comments
fixed unused imports
@ahmed-arb ahmed-arb changed the title add author, book and loan models add core app Jan 17, 2023
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