You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
After struggling with the undocumented, untested code base that was mostly written by others than @redekok and me (think of the EnergySystemHandler, ETM_API and such) once again yesterday, I finally became fed up with it. Sorry again that my ETEngine commits took the fall for that @antw.
So, here's my pitch to restructure the entire app!
First off I would really like to add a test suite. Normally easily done by using pytestand loading the app in as a fixture. Trying this out I came to the conclusion that the entire structure of the app is.. well.. not even following the first step of the how-to-build-a-basic-flask-app tutorial. Meaning we can't load up the app in the test suite right now. My guess is that this app was set up as a bit of a playground - not to be a solid application.
What's needed:
Create a Flask factory with app configs and routes, separated from the controllers etc
Setup a basic test suite
Move all api controllers to their own file and put them in a nice folder
Add tests for all api controllers
Review the scripts that are in helpers and put them in more fitting folders (including interface.py)
Setup test integration on github/semaphore
Set Roos up with a testing environnement for the conversion methods in interface.py
Remove the data folder
Add tests for all the files/classes that were in helpers
The first four + the integration of tests are the most pressing IMO. I think that can be done 4-5 days with some help from @redekok, as we only have 2 controllers at the moment.
The other three are also important, but maybe as a second phase if we don't have time/budget right now. I expect 7 and 9 to both be effort:high as well.
This restucture will not only help us right now, but I think having a solid test suite will also help Roos in adding new assets to the etm-esdl conversion in the future (see 7). I we want to be serious in this ESDL business, this app needs some love.
@ChaelKruip@redekok what do you think of this restructure idea, do we have time/budget for it?
The text was updated successfully, but these errors were encountered:
This sounds like an excellent plan @noracato! I love the idea for a restructure including a testing environment. That will definitely save us lots of frustration. Let's discuss the possibilities for time and budget offline.
After struggling with the undocumented, untested code base that was mostly written by others than @redekok and me (think of the
EnergySystemHandler
,ETM_API
and such) once again yesterday, I finally became fed up with it. Sorry again that my ETEngine commits took the fall for that @antw.So, here's my pitch to restructure the entire app!
First off I would really like to add a test suite. Normally easily done by using
pytest
and loading the app in as a fixture. Trying this out I came to the conclusion that the entire structure of the app is.. well.. not even following the first step of the how-to-build-a-basic-flask-app tutorial. Meaning we can't load up the app in the test suite right now. My guess is that this app was set up as a bit of a playground - not to be a solid application.What's needed:
helpers
and put them in more fitting folders (includinginterface.py
)interface.py
data
folderhelpers
The first four + the integration of tests are the most pressing IMO. I think that can be done 4-5 days with some help from @redekok, as we only have 2 controllers at the moment.
The other three are also important, but maybe as a second phase if we don't have time/budget right now. I expect 7 and 9 to both be effort:high as well.
This restucture will not only help us right now, but I think having a solid test suite will also help Roos in adding new assets to the etm-esdl conversion in the future (see 7). I we want to be serious in this ESDL business, this app needs some love.
@ChaelKruip @redekok what do you think of this restructure idea, do we have time/budget for it?
The text was updated successfully, but these errors were encountered: