Closed BenHargreaves closed 6 years ago
Generally it looks good and I like the simplicity of the approach.
However, don't you think that the generateEmptyJson()
should be called from main? On the other hand I like that main.py and updater.py are independent right now.
If you have any ideas on this let me know, otherwise we can merge this as is
Thanks for taking a look @zachkont
The call to generateEmptyJson()
on main.py to initialize the JSON files would be unnecessary because of how the utils.py file is structured. When we Import the utils.py module into main and updater, All the code in utils.py gets executed by default (which already contains a call to generateEmptyJson) - we would have to throw in a if __name__ == '__main__':
check in utils to check that the file is being run directly vs being imported if we wanted to change that behaviour.
I could see value in adding a call to generateEmptyJson()
at the start of the loadJson()
method in utils though - this would help with errors being thrown if the json files get deleted manually after main and updater are already running.
Adding the generate()
at the start of loadJson()
sounds good, let's do that
Sure thing - Updated the PR.
I also created a separate PR to update the Readme. The manual JSON file creation wont be necessary after this is merged in
Readme PR - #58
Your checklist for this pull request
🚨Please review the guidelines for contributing to this repository.
Description
Feature fix for #39
Added a method in utils.py which checks for the required JSON files - and creates them if they dont exist. This Method will be called any time utils.py is loaded, but the check for the file already existing should prevent them from being overwritten
NOTE!! - I needed to add a reference to the os.path module for the 'file check' to work. If you think there is no need for this check to exist, I can remove the reference or figure something else out :)
💔Thank you!