openplans / shareabouts

Shareabouts is a mapping application for crowdsourced info gathering.
GNU General Public License v3.0
278 stars 149 forks source link

Unused requirements in frontend Shareabouts #213

Closed BenSturmfels closed 1 year ago

BenSturmfels commented 1 year ago

Looking at the frontend Shareabouts requirements.txt I see that it includes shareabouts-api and then lists a bunch of dependencies that are really only related to the backend codebase. This seems like unnecessary duplication. You COULD deploy them into the same virtualenv, but there's no real need to (we reuse the same backend for many separate projects, so don't share a virtualenv).

Perhaps the API requirements could be maintained in shareabouts-api/setup.cfg so that they are installed automatically.

OR

Perhaps the backend and its requirements should be something you install separately and could be removed from the frontend requirements.txt. For example, removing packages like:

What do you think?

BenSturmfels commented 1 year ago

(The motivation behind this is that I was looking at #207 and wondering why that seemingly backend-related installation issue was being raised in the frontend shareabouts project.)

BenSturmfels commented 1 year ago

The other awkward thing about having the dependencies duplicated in both frontend and backend is that if we wanted to for example, upgrade Django, both projects would need to be upgraded in lock-step. And if they're coupled like this, they probably should be in a single repository. Not really suggesting using a single repository - just that it's awkward if they can't be upgraded independently.

mjumbewu commented 1 year ago

I totally agree with this. I think it was an unnecessary complication to have a version of the requirements that included the API server. As you mentioned, someone could just install them separately if they really wanted to run both on the same machine.