search

creativecommons / cc-legal-tools-app

Legal tool (licenses, public domain dedication, etc.) management application for Creative Commons
https://creativecommons.org/licenses/
MIT License
84 stars 85 forks source link

[Feature] Improve the "Setting up..." instructions for less-technical users #468

Closed briandominick closed 1 month ago

briandominick commented 1 month ago

Problem

The current setup instructions work fine for anyone without Docker yet installed, but they treat Docker installation and Git cloning as simple steps. The current Docker Engine link is no longer ideal, and may be confusing for Windows/MacOS users. Docker Engine is now only available for Linux and should not be installed on WSL2.

Description

There's no reason to label this the "Docker Compose Setup" as the only other setup instructions are not recommended and require Docker Compose setup anyway. Rename it "Application Setup".

Move the Docker installation instructions step to a longer note under a Prerequisites subsection. Git is also a prerequisite that should not be presumed since the steps require cloning. Docker instructions should state that Compose v2 is required (as it seems to be from my testing; docker-compose CLI throws errors where docker compose does not). Use the correct links to Docker apps, advising Docker Desktop for Windows/WSL2 users (https://docs.docker.com/desktop/wsl/), Docker Engine for Linux users (https://docs.docker.com/engine/install/), and Docker Desktop for MacOS users (https://docs.docker.com/desktop/install/mac-install/).

More explicitly instruct the process for cloning both interdependent repos side by side, then cd'ing into the -app repo.

Remove: "Pleaes note that CC staff use macOS for development--please help us with documenting other operating systems if you encounter issues."

Add a note that default local.py settings should work fine locally. (Or else advise context-specific alterations local/dev users may need to make???)

I think Step 7 "Initialize data" needs to be performed in a separate terminal tab since containers are running and must be running for this to work? We should note this.

Should we also consider recommending a --depth 1 clone for the cc-legal-tools-data repo to speed up?

Alternatives

The main alternative I considered was recommending moving all of this to more polished docs under opensource.creativecommons.org, but I think the better idea will be to publish a single-sourced version of these instructions (and maybe other parts of the README) at a later date.

Implementation

TimidRobot commented 1 month ago

I like the idea of all of our repositories having clearly documented assumptions/prerequisites. I also think that technologies that are not repository specific should ideally be represented by a single link to an authoritative source. For example, I don't want repository READMEs to have detailed instructions on using git or GitHub.

briandominick commented 1 month ago

I've started work on this ticket, choosing it first because it's a contained section (or so) that we can rearrange when implementing:

I think I have found a problem that I'd be happy to report as a bug, but I'm not sure I'm getting it right.

In this section:

If it is not cloned into the default location, the Django DATA_REPOSITORY_DIR Django configuration setting, or the DATA_REPOSITORY_DIR environment variable can be used to configure its location.

I think the COMPOSE_FILE variable in .env is causing these instructions not to work. It doesn't seem that docker compose up (or maybe even build?) will run if the data repo is not at exactly the default path, even if the environment variable is changed in the shell or if you hard-code it into the Django config (which also seems ill-advised).

I believe the above-quoted instruction is suggesting one change DATA_REPOSITORY_DIR in cc_legal_tools/settings/base.py or else change the shell var with export DATA_REPOSITORY_DIR='./alternate/path' (or something more permanent), correct?

If I'm right, the former instruction is actually pretty complicated and not for anyone who probably couldn't figure it out to attempt. But in any case, if the data directory is not in the default directory, I think 2 settings have to be changed in .env (COMPOSE_FILE and RELPATH_DATA) as well as we still need a shell env var to be set, for Django (and possibly other processes?) to pick up.

So I'm wondering if maybe we should remove this instruction for now, as it doesn't seem to actually work and it's not very useful as is, potential bugs aside.

TimidRobot commented 1 month ago

@briandominick yes, i think removing that instruction is helpful.

It's confusing because there are two layers: 1) .env:RELPATH_DATA indicates where the source directory is that is mounted inside the Docker container 2) cc_legal_tools/settings/base.py:DATA_REPOSITORY_DIR is for where the running application looks (assumed to be running inside the docker container)

It should be possible to change RELPATH_DATA without much fuss, but changing DATA_REPOSITORY_DIR would also require a change to docker-compose.yml.

I haven't tested it recently because I don't think there's any value in modifying these variables. The use case of this app is very specific. Removing the instruction is a good call.