Skip to content
This repository has been archived by the owner on Sep 24, 2019. It is now read-only.

utils.py generates required Json files when called #56

Merged
merged 2 commits into from
Oct 30, 2018

Conversation

BenHargreaves
Copy link
Contributor

@BenHargreaves BenHargreaves commented Oct 19, 2018

Your checklist for this pull request

🚨Please review the guidelines for contributing to this repository.

  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side). Don't request your master!
  • Make sure you are making a pull request against the development branch (left side). Also you should start your branch off our development.
  • Check the commit's or even all commits' message styles matches our requested structure.
  • Check your code additions will fail neither code linting checks nor unit test.

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!

@zachkont
Copy link
Owner

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

@BenHargreaves
Copy link
Contributor Author

BenHargreaves commented Oct 22, 2018

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.

@zachkont
Copy link
Owner

Adding the generate() at the start of loadJson() sounds good, let's do that

@BenHargreaves
Copy link
Contributor Author

BenHargreaves commented Oct 25, 2018

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

@zachkont zachkont merged commit 7849f99 into zachkont:development Oct 30, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants