search

ODM2 / WOFpy

A server-side implementation of CUAHSI's Water One Flow service stack in Python.
http://odm2.github.io/WOFpy/
9 stars 9 forks source link

Delete apps/templates not used anywhere. #123

Closed lsetiawan closed 7 years ago

lsetiawan commented 7 years ago

apps/templates is not used and not included within the package. This PR removes the whole apps/templates folder.

emiliom commented 7 years ago

Thanks. FYI, I know this PR doesn't impact what you're doing today, so I'll get back to this late in today, possibly tomorrow.

emiliom commented 7 years ago

@valentinedwv, we've found that the "templates" folder is present (duplicated) in two places:

The latter seems to have slightly more recent versions of the files and is being used; the former doesn't seem to be used by wofpy anywhere. This PR will delete the former. But before we do that, can you take a look and see if this duplication rings any bells and it's ok to delete wof/apps/templates? Just trying to be extra sure we're not overlooking something.

Thanks.

emiliom commented 7 years ago

@lsetiawan, I happened to be looking at examples/flask/odm2/timeseries/odm2_config_timeseries.cfg and noticed that the WOFpy/Templates variable points to wof/apps/templates. You alluded to this yesterday, here. But more generally, to avoid confusion to users, it seems it'd be best to edit all the DAO cfg files so they point to wof/flask/templates instead, right -- or at least the DAO's we've actively looked at, and that are in the CI test suite? I realize users will be instructed to edit that variable anyway, but it's cleaner to remove all references to wof/apps/templates if we're removing it completely.

Does this make sense?

Maybe add the commits for those changes to this PR?

Thanks.

lsetiawan commented 7 years ago

But more generally, to avoid confusion to users, it seems it'd be best to edit all the DAO cfg files so they point to wof/flask/templates instead, right -- or at least the DAO's we've actively looked at, and that are in the CI test suite? I realize users will be instructed to edit that variable anyway, but it's cleaner to remove all references to wof/apps/templates if we're removing it completely.

Thanks for catching this! Yea. This was set to apps/templates because I was confused before and changed it to this. Now I know that it's not used. I will definitely change it to flask/templates

emiliom commented 7 years ago

Thanks!

emiliom commented 7 years ago

I'll go ahead and merge.