feat: add files needed for copier app #11
Conversation
There was a problem hiding this comment.
Overall LGTM! Thanks for the improvements you've made to this app/config - I think it'll make the artifact repo review process clearer and easier.
Couple of questions to flag for consideration, but nothing blocking - just a question of how we want to coordinate with planned dir name changes.
|
|
||
| # Configure PR metadata for batched PRs with accurate file counts | ||
| batch_pr_config: | ||
| pr_title: "Update MFlix application from docs-sample-apps" |
There was a problem hiding this comment.
Just noting here for posterity that this (referring specifically to MFlix app) is all good for now, but in the future, when we add other apps, we may want to make this more generic.
Although I guess we could just rename it to "sample application" here, in line 15, and in the commit message to be forward-proof in the PR description.
We don't currently have plans for any other sample apps so I'm pretty impartial to referring to MFlix by name - up to you whether you want to change it.
| - name: "mflix-express-server" | ||
| source_pattern: | ||
| type: "regex" | ||
| pattern: "mflix/server/express/(?P<file>.+)$" |
There was a problem hiding this comment.
IIRC we talked about changing this to javascript-express to match the language-server pattern some of the other apps use. So just wanted to flag this, and ask if we want to wait until that change happens and change this pattern to match, or merge this as-is and change the pattern later.
| - name: "mflix-python-server" | ||
| source_pattern: | ||
| type: "regex" | ||
| pattern: "mflix/server/python/(?P<file>.+)$" |
There was a problem hiding this comment.
Same thing here - I didn't know if we wanted to wait to merge this until we rename this to python-fastapi and change the pattern in this PR, or if you want to merge this as-is and then change the pattern when we update the filepath.
No description provided.