Marina_Romanchuk_marina-romanchuk-2015@mail.ru#27
Conversation
|
Here is some feedback on your app work:
|
|
Try to rethink logic in your small functions a bit. Right now they are too large |
HenadziStantchik
changed the title
mrskmrn
changed the title
Same for HTML converted file
|
HenadziStantchik
changed the title
| For doing that you should enter directory, and if it's correct, news will be converted and you will see a message, | ||
| which tells, if converting was successful or not. | ||
| In prepared file (it is named news.pdf or news.html) will be all information about news, if there is no connection | ||
| to the Internet, instead of image there will be url of it. No newline at end of file |
There was a problem hiding this comment.
Please add some info about the way you store files
| @@ -0,0 +1 @@ | |||
| include rss_reader/ARIALUNI.ttf No newline at end of file | |||
There was a problem hiding this comment.
This will not work on Windows OS, because is has different filesystem path separators (\)
| return {"url": source, | ||
| "lim": limit, | ||
| "json": json, | ||
| "date": date, | ||
| "path": path, | ||
| "html": to_html, | ||
| "pdf": to_pdf} |
There was a problem hiding this comment.
It would be better just to use
{
"url": args.source,
...
}
Instead of creating redundant variables.
| print("Invalid directory") | ||
| logging.error("Directory doesn't exist") | ||
| return None | ||
| filename = os.path.join(path_to_file, "news." + expansion) |
There was a problem hiding this comment.
It is better to use type or format instead of expansion
| filename = get_path(dict_of_args.get("path"), "html") | ||
| if not filename: | ||
| return None | ||
| file = open(filename, 'w') |
There was a problem hiding this comment.
It is better to use context manager for opening and closing file.
| from console_interface import parse_args | ||
| import logging | ||
| import sqlite3 | ||
| from rss_parser import * |
There was a problem hiding this comment.
Here and below: it is better to always avoid wildcard import
| description='Pure Python command-line RSS reader.', | ||
| author='Marina Romanchuk', | ||
| author_email='marina-romanchuk-2015@mail.ru', | ||
| data_files=[('rss_reader', ['rss_reader/ARIALUNI.ttf'])], |
There was a problem hiding this comment.
Do you actually need this here? You have this file specified in your MANIFEST.in
| @@ -0,0 +1,28 @@ | |||
| import sys | |||
| sys.path.insert(1, 'final_task/rss_reader') | |||
There was a problem hiding this comment.
sys.path.append will work here too, there is no need to insert it to the second position of the list.
|
Here is feedback on your application: Some tests fail. Coverage percentage is not high enough. |
3 participants
No description provided.