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

feat: add django-diagrams hook #9

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions .pre-commit-hooks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,9 @@
entry: (has_perm|@permission_required)\(["'][^\."']+["']\)
language: pygrep
types: [python]
# when calling this hook in a repo you must add args: [<path to local settings e.g. qrhub.settings.local>] in the repo .pre-commit-config.yaml file
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use the DJANGO_SETTINGS_MODULE env var?

Copy link
Author

@Gen-QR Gen-QR Jan 9, 2024

Choose a reason for hiding this comment

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

It would be better if we could but I couldn't find a way to get it into the .pre-commit-hooks.yaml file

Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to use the env var, otherwise it won't be able to use the correct settings in both local and CI contexts. Not sure what you mean by getting it into the YAML file, but first thing to try would be echo $DJANGO_SETTINGS_MODULE in the bash script to see if it's set.

Copy link
Author

Choose a reason for hiding this comment

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

I tried that but will just double check

- id: create-django-diagrams
name: Create Django Diagrams
entry: bash generate_diagrams.sh
language: system
files: ^.*/migrations/.*\.py$
34 changes: 34 additions & 0 deletions generate_diagrams.sh
dave-v marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#!/bin/bash

DIRECTORY="./docs/model_diagrams/"

# If the directory doesn't exist, create it
mkdir -p $DIRECTORY

# the settings module is the first argument
SETTINGS_MODULE="$1"

# remove the first argument passed to the script
shift

# the rest of the arguments are migration file names
MIGRATIONS=$@
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't this be empty?

Copy link
Author

Choose a reason for hiding this comment

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

No it shouldn't. The hook passes the migration file names as args (at least testing locally on the hub it does. It should behave the same way here)

Copy link
Contributor

@dave-v dave-v Jan 8, 2024

Choose a reason for hiding this comment

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

I see, then presumably it is passing all changed files and you need to filter out those that are not migrations? Or does the pre-commit files parameter do that filtering for you?

Copy link
Author

Choose a reason for hiding this comment

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

The pre-commit files param does it


# declare an array to hold the app names
declare -a APP_NAMES=()

# loop through the migrations and get the app names
for migration in $MIGRATIONS; do
APP_NAMES+=("$(echo "$migration" | cut -d'/' -f1)")
done

# declare an array and use it to get the unique app names
FINAL_APP_NAMES=($(echo "${APP_NAMES[@]}" | tr ' ' '\n' | sort -u | tr '\n' ' '))

# loop through the unique app names and generate the diagrams
for APP in "${FINAL_APP_NAMES[@]}"; do
python -m django_diagram -a $APP -o $DIRECTORY/$APP.md -s $SETTINGS_MODULE
done

# add the diagrams to git
git add $DIRECTORY