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

Issues provided by Peixin Li #17

Open
Irislpx opened this issue Dec 3, 2017 · 0 comments
Open

Issues provided by Peixin Li #17

Irislpx opened this issue Dec 3, 2017 · 0 comments

Comments

@Irislpx
Copy link

Irislpx commented Dec 3, 2017

This is a new issues report after last issues provided a few weeks ago. I noticed that there are several commits after last code review which is really good , meaning that you are keep working on your project.

There are some checklist for me to do this code review.

- Code formatting

I used online PEP8 checking website to check if your code meet with PEP8 requirement. Results are shown below of login_register.py file:
screen shot 2017-12-03 at 11 21 24 am

So, as you can see, you should do some modification about your code format.

- Architecture

It looks good to me since there are basically three layers that work for each other. But I cannot find what Local_Web_UI really looks like because there isn't any screenshots for me to review. Hope you can add them later to better illustrate your project. Besides When I looked into your websites, I noticed that websites named http://medusapys.site broke down for some reasons, while another one worked good. However, I cannot use your web application without signing up and I only found sign in page which still cannot help me to register and use your websites. I checked code about regiter.html and app.py, then I realized that you didn't have any action linked when you send post request in the first file

screen shot 2017-12-03 at 11 52 54 am

and it lacks some routers about register and login in second file respectively.

screen shot 2017-12-03 at 11 54 00 am

- Coding best practices

There do exist some hard coding in your website pages, please use constants/configuration values.

This is my code review, hope it can help!

@Irislpx Irislpx changed the title Issued provided by Peixin Li Issues provided by Peixin Li Dec 6, 2017
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

No branches or pull requests

1 participant