Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: update docs since TACC/Core-CMS#741 #755

Merged
merged 3 commits into from
Nov 28, 2023

Conversation

wesleyboar
Copy link
Member

@wesleyboar wesleyboar commented Nov 28, 2023

Overview / Changes

  1. Update docs since Update dockerfile to speed up builds #741 altered local development.
  2. Minor improvements to docs.

Related

Testing

I author from recent memory of repeat actions done often during development and testing.

UI

N/A

Copy link
Member Author

@wesleyboar wesleyboar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notes for reviewers.

* [Docker Engine] ≥ v20
* Docker Engine ≥ v20
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cruft. Nothing to link to. Docker Engine is installed via Docker Desktop.

Comment on lines +73 to +75
| For Testing | For Developing & Testing |
| - | - |
| `make start` | `docker-compose -f ./docker-compose.dev.yml up` |
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some development still requires docker-compose -f ./docker-compose.dev.yml1 e.g. Locally Develop CMS and Styles.

Footnotes

  1. Eventually, I might be able to replace it with make start-dev, but not yet.

Comment on lines -85 to +89
5. Run the Django Application:
5. Update the Django Application:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The application is already running via make start or docker compose … up above.


4. Enter the CMS Docker Container:

(This opens a command prompt within the container.)

```sh
docker exec -it core_cms /bin/bash
# This opens a command prompt within the container
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just being consistent with other instances of this command in isolation.

Comment on lines -121 to +142
- If CMS `Dockerfile` changed, rebuild Docker Containers:

```sh
make stop
make build
make start
```

- If anything else changed, update the Django application:

```sh
docker exec -it core_cms /bin/bash
# This opens a command prompt within the container.
```

Run relevant commands within the container:
#### For Testing

- If **styles** changed:
```sh
make stop
make build
make start
```

```sh
npm ci
npm run build:css --project="core-cms"
python manage.py collectstatic --no-input
```
#### For Development & Testing

- If **assets** changed:

```sh
python manage.py collectstatic --no-input
```

- If **models** changed:

```sh
python manage.py migrate
```
| If this changed | Run this command |
| - | - |
| Dockerfile | `npm ci` |
| Node dependencies | `npm ci` |
| CSS stylesheets | `npm run build:css` |
| UI Demo | `npm run build:ui-demo` |
| assets e.g.<br><small>images, stylesheets, JavaScript</small> | `docker exec -it core_cms sh -c "python manage.py collectstatic --no-input"` |
| Python models | `docker exec -it core_cms sh -c "python manage.py migrate"` |
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope this:

  • makes use cases clearer
  • saves time in the terminal
  • teaches what commands do

Comment on lines -43 to +36
3. Build Assets:
2. Build Styles:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No assets will ever be built. Only styles.

I had thought maybe they would be, but no. In fact, I plan to remove even building of styles, thus all of Node, from Core-CMS.

Comment on lines -27 to -32
1. Enter Container:

```sh
docker exec -it core_cms /bin/bash
# This opens a command prompt within the container.
```
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to enter container in isolation, because only one command need be run in container now, and it can be run inside the container from a command outside the container.

Comment on lines -54 to +56
docker exec -it core_cms /bin/bash
# That opens a command prompt within the container.
python manage.py collectstatic --no-input
docker exec -it core_cms sh -c "python manage.py collectstatic --no-input"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just making this a one-liner, now that I know how.

docker exec -it core_cms /bin/bash
# That opens a command prompt within the container.
npm run build:ui-demo
npm run build:ui-demo
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THIS IS A FUNCTIONAL CHANGE! We cannot run npm inside container since #741.

taccsite_custom Outdated
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar updates and other improvements. See TACC/Core-CMS-Resources#196.

@wesleyboar wesleyboar marked this pull request as ready for review November 28, 2023 20:20
@wesleyboar wesleyboar merged commit eb927c5 into main Nov 28, 2023
@wesleyboar wesleyboar deleted the docs/update-docs-since-pr-741 branch November 28, 2023 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant