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

Automated ER diagram generation #83

Merged
merged 12 commits into from
Dec 21, 2024

Conversation

Scilly-guy
Copy link
Contributor

Added make command to refresh entity relationship diagram.

@Scilly-guy
Copy link
Contributor Author

Suggest updating read me to include instructions to install graphviz to enable ER diagram generation.

Copy link
Member

@grundleborg grundleborg left a comment

Choose a reason for hiding this comment

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

This looks good. I have a couple of additional requests:

  1. Please can you add instructions (and a note on additional dependencies) to the README.md file.
  2. Please can you add all the files generated by this script to .gitignore so that they don't get accidentally checked in to the repo.
  3. To make the above simpler and keep things tidier, I think you should create a subdirectory for the entity relationship diagram stuff, and move the insert-diagram.py and template_entity_relatoinship_diagram.html files into it, as well as adjusting the generation commands to create all their files within that directory too.
  4. You've added a dependency which means you'll need to run poetry lock (I think) to update poetry.lock file.

insert-diagram.py Outdated Show resolved Hide resolved
template_entity_relationship_diagram.html Outdated Show resolved Hide resolved
Copy link
Member

@grundleborg grundleborg left a comment

Choose a reason for hiding this comment

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

Fixed a couple of minor things when testing on my computer, but this LGTM now.

@grundleborg grundleborg merged commit eae1c44 into ios-cv:main Dec 21, 2024
1 check passed
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.

2 participants