-
Notifications
You must be signed in to change notification settings - Fork 80
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
Removed folders that are empty. #80
Conversation
I will check out your PR and merge it if it's ok. Thanks alot for your help. It's really appreciated. |
Option number (1) was fixed by @KokoseiJ. I just built on top of it. |
Nice, Thank you very much. |
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.
Just a quick review. I'm not a professional dev but those parts might need some changes.
@@ -3,6 +3,7 @@ | |||
import os | |||
from cronController import schedule_end, schedule_start | |||
from organiseDesktop import undo, organise_desktop | |||
from os import path, mkdir, listdir, rename, environ, rmdir |
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.
This might be a bit of an overreaction, but Wouldn't it be better to just use os module instead of importing those functions again, just like other contributors did, for the sake of consistency of the code style?
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.
Yep, you are right, or just import the needed ones but not both. Give it a go :)
#78 has been resolved.
@blavejr both the suggestions,
have been solved.