Skip to content

feat(scripts): update Makefile commands and script folders #12

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions Makefile
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have a Makefile at docussaurus/Makefile, remove that, as this file does the same thing.

Original file line number Diff line number Diff line change
@@ -1,17 +1,24 @@
.PHONY: dev precommit precommit_fix help
.PHONY: dev build start precommit precommit_fix sidebar_check help

dev: ## Start local docs server with live reload
make -C docusaurus dev
ENV ?= development

start: ## Start Docusaurus site (default: ENV=development)
ENV=$(ENV) bash scripts/create_docusaurus_website.sh

dev: ## Alias to start in development mode
ENV=development bash scripts/create_docusaurus_website.sh

build: ## Alias to start in production mode
ENV=production bash scripts/create_docusaurus_website.sh
Comment on lines +3 to +12
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you get the ENV from environment, you don't need to separate dev from production, call it docussaurus or leave as default, calling only make to build and run docussaurus, usually this is done in a all section in the makefile. And update documentation


precommit: ## Run docs precommit checks
make -C readmes precommit

precommit_fix: ## Try to fix existing precommit issues
make -C readmes precommit_fix

sidebar_check: ## Check if all pages are implemented with sidebars for 1.7.0, 1.8.0, and latest
python3 check_sidebars.py
sidebar_check: ## Check if all pages are implemented with sidebars
python3 scripts/check_sidebars.py

# Ref: https://marmelab.com/blog/2016/02/29/auto-documented-makefile.html
help: ## Show documented commands
@grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-25s\033[0m %s\n", $$1, $$2}'
10 changes: 9 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,12 @@ A standalone home for work on documentation for Magma, in order to make check-in
This document provides pointers for those looking to make documentation changes for the Magma project

- [Documentation Overview](https://github.com/magma/magma/wiki/Contributing-Documentation) for general documentation information
- `make help` for specific commands
- Makefile usage:
```bash
make dev # starts Docusaurus with live reload (dev mode)
make build # starts Docusaurus in production mode
make start ENV=production # also possible
make precommit # runs precommit checks
make sidebar_check # checks if all docs have a sidebar entry
make help # shows all available commands
```
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ original_id: docusaurus
### Generating the Documentation Website

1. Ensure [docker](https://docs.docker.com/install/) is installed
2. From `magma/docs`, run `./docusaurus/create_docusaurus_website.sh`
2. Run `./scripts/create_docusaurus_website.sh`
3. Navigate to http://127.0.0.1:3000/magma/ to view a local version of the site

### Directory Structure
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ original_id: docusaurus
### Generating the Documentation Website

1. Ensure [docker](https://docs.docker.com/install/) is installed
2. From `magma/docs`, run `./docusaurus/create_docusaurus_website.sh`
2. Run `./scripts/create_docusaurus_website.sh`
3. Navigate to http://127.0.0.1:3000/magma/ to view a local version of the site

### Directory Structure
Expand Down
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add execution permission to this file.
Running ./scripts/check_sidebars/py returns nothing.
Script is not working, fix the execution path.

File renamed without changes.
49 changes: 35 additions & 14 deletions docusaurus/create_docusaurus_website.sh → scripts/create_docusaurus_website.sh
100755 → 100644
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add execution permission to this file

Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,23 @@

set -e

# ==============================
# Choose environment: development | production
# Default is "development"
ENV=${ENV:-development}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Receive as argument instead of environment variable.

# ==============================

function exit_timeout() {
echo ''
docker compose logs docusaurus
docker compose logs $SERVICE
Copy link
Collaborator

Choose a reason for hiding this comment

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

Variable $SERVICE not previously set. Prefer passing arguments, makes syntax easier to understand.
You may do SERVICE=${2:default_service} and use as is and call exit_timeout time some_service.

echo ''
echo "Timed out after ${1}s waiting for Docusaurus container to build. See logs above for more info."
echo "Possible remedies:"
echo ' - Remove node_modules directory (rm -rf node_modules) and try again.'
exit 1
}

# spin until localhost:3000 returns HTTP code 200.
# spin until localhost:3000 returns HTTP code 200 (only for dev)
function spin() {
maxsec=300
spin='-\|/'
Expand All @@ -38,16 +44,31 @@ function spin() {
printf "\r \n"
}

# Select service name and port based on environment
if [[ "$ENV" == "production" ]]; then
SERVICE="docusaurus-prod"
PORT=8080
URL="http://localhost:$PORT/"
else
SERVICE="docusaurus-dev"
PORT=3000
URL="http://localhost:$PORT/docs/basics/introduction"
fi
Comment on lines +47 to +56
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like the approach to assume a single deployment mode, or at least assume production first.
Are you comfortable to assume a "production always" deployment?
This way, we don't incur the risk of testing something in dev mode and forgetting to configure the equivalent in the production mode. We might have a dev deployment, but I would feel more comfortable if it was an extension of the production, not a different scenario.

For production:

  • the URL configured as localhost might present some issues, 0.0.0.0 is better, because listen in all configured interfaces instead of only the loopback.
  • the protocol used is HTTPS, not HTTP, HTTPS implements a layer of security and exchange of certificates.
  • the port used is the 443, the default for HTTPS.
  • you might still use the docussaurus in HTTP, but reverse proxy is needed, nginx is commonly used for that, it also adds the security layer needed for the HTTPS.


# Run
echo "==> Starting Docusaurus in '$ENV' mode..."

docker compose down
docker build -t magma_docusaurus .
docker compose --compatibility up -d

echo ''
echo 'NOTE: README changes will live-reload. Sidebar changes require re-running this script.'
echo ''
echo 'Waiting for Docusaurus site to come up...'
echo 'If you want to follow the build logs, run docker compose logs -f docusaurus'
spin
echo 'Navigate to http://localhost:3000/ to see the docs.'

xdg-open 'http://localhost:3000/docs/next/basics/introduction.html' || true
docker compose --compatibility up -d $SERVICE
Copy link
Collaborator

Choose a reason for hiding this comment

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

Compose file is not accessible running ./scripts/create_docussaurus_website.sh


if [[ "$ENV" == "development" ]]; then
echo ''
echo 'NOTE: README changes will live-reload. Sidebar changes require re-running this script.'
echo ''
echo 'Waiting for Docusaurus site to come up...'
echo "If you want to follow the build logs: docker compose logs -f $SERVICE"
spin
fi

echo "==> Docusaurus is running at: $URL"
# xdg-open "$URL" || true
Comment on lines +47 to +74
Copy link
Collaborator

Choose a reason for hiding this comment

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

Create a main function is a good practice, but you could break this into more functions.
At the end, you call main.