-
Notifications
You must be signed in to change notification settings - Fork 2
Add ADR for renaming templates #21
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
base: main
Are you sure you want to change the base?
Conversation
|
||
## Decision Outcome | ||
|
||
1. Use a truncated version of the template application name (e.g. `app-nextjs` for `template-application-nextjs`, `app-flask` for `template-application-flask`) for the application directory |
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.
My mundane POV is that this feels like a good place to use flask-app
rather than app-flask
.
Besides that, I'm hard +1 on renaming away from just app
, for all the reasons mentioned here
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.
Thanks for the comment! I tend to follow "bigger-to-smaller" in most things (e.g. YYYY-MM-DD) so that things naturally sort, which is how we ended up with template-application-flask
, but I don't feel super strongly about it. Curious to see what other people think.
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 didn't have an opinion but Rocket's explanation makes sense to me.
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 like the idea of naming the app something that is easier to find-and-replace
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.
At a high level seems like a reasonable direction, but I'm having difficulty evaluating without seeing the impact on:
- Setup process for integrating apps and infra (right now infra assumes apps are named
app
, so there needs to be an additional step that updates the infra template to get them to work) - Update process for updating the templates (both app and infra)
- Impact on template development workflows, in particular (a) template CD which uses update process and (b) dev workflow which often relies on generating a patch from changes in a branch on platform-test and applying them to a branch on template-infra.
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.
Read this over in detail and makes sense to me, like the idea of avoiding app/app in next.js
Ticket
Changes
Context for reviewers
See ADR.
Testing
N/A