-
Notifications
You must be signed in to change notification settings - Fork 1
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
[ETL-574] Add CONTRIBUTING.md #91
Conversation
CONTRIBUTING.md
Outdated
1. Clone this repository (do not fork). Create a feature branch off the `main` branch. Branch names must contain the identifier of the corresponding Jira ticket (e.g., `etl-574`). Optionally, add keywords describing the work that is being done to the branch name (e.g., `etl-574-add-contributing-doc`). | ||
|
||
> [!IMPORTANT] | ||
> Keep branch names as short as possible. We use the branch name to namespace resources in AWS (as described below). Since AWS imposes limits on the name length of some resources, the deployment may fail if the branch name is too long. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC capital letters in the branch name also causes issues
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find a good reference for safe characters in AWS resource names, so I added a generic disclaimer about using safe characters ([a-z], hyphen, underscore)
CONTRIBUTING.md
Outdated
pipenv install --dev | ||
``` | ||
|
||
from the repository root to create a virtual environment and install all dependencies. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if it's possible to put code blocks at an indent level, so I moved the code block to the bottom of point 2 so the formatting is less jarring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes LGTM! Great work on the documentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥 Fantastic! I just added some comments - but pre-approving!
Things to look out for: