-
Notifications
You must be signed in to change notification settings - Fork 11
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
[NEW FEATURE][WORK IN PROGRESS]Implement Basics for a Web App #106
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @MatejMecka . I know this is a WIP but still below are some changes I would like you to implement.
- Keep tests for web in the modular way only in the tests folder in root of the repo.
- Write tests for all your function (almost).
- Write docstrings in the same manner as the other modules (you're doing great on this already).
- Use the current address of the database and do not create a new one.
I've not run it and hence some points may not be accurate.
You're doing great 👍
Codecov Report
@@ Coverage Diff @@
## master #106 +/- ##
=======================================
Coverage 83.79% 83.79%
=======================================
Files 26 26
Lines 1413 1413
Branches 107 107
=======================================
Hits 1184 1184
Misses 184 184
Partials 45 45 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are few things that are required to be done before I test this;
- Add all dependencies in the Pipfile; I did not find flask_testing nor flask_sqlalchemy.
- Update documentation regarding how to run tests. (I forgot to add this).
- Update your importing style to be relative (as I've used everywhere).
- Show me the screenshot of the web app running.
- Include documentation for WebApp on the wiki (and also hint a bit in readme.md)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Below are some changes and improvements I want:
- Add all dependencies to Pipfile and update it.
- Use relative importing (as I've used everywhere) in your code as well.
- Attach a screenshot of the web app.
- Update documentation regarding webapp on the wiki and add some info about it in readme.md as well
- Add instructions on how to run tests in the readme.
While doing all above tasks follow the method already used :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please implement the suggested changes :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thanks for showing interest in the project and sending us a pull request.
Please prefix your pull request with [NEW FEATURE], [BUG] or [IMPROVEMENT]
What issue/feature is fixed?
#99 I am creating a Flask Handler with tests and documentation
Issue/Feature Details
You can mention the line of code that is causing the error, detail the feature you are requesting and where it can be useful.
N/A
How To Test It Locally?
cd Nephos/nephos/web
python3 webServer.py
Visit in your browser