-
Notifications
You must be signed in to change notification settings - Fork 0
#94 lambda readme generation #111
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
…n't get lost when regenerated
| routes = [{ method: 'GET', path: '/health', description: 'Health check' }]; | ||
| } | ||
|
|
||
| const description = options.description || `TODO: Add a description of the ${handlerName} lambda.`; |
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.
When a new lambda is generated, it will be on the developer of that PR to update the description (if it's regenerating for an old lambda the description won't change). Is this fine or do we want to use another template description to reduce the number of potentially forgotten todo's (like "Lambda for ${handlerName}")
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.
Good question, I think its ok to leave the TODO, hopefully either the dev or one of the reviewers will catch it.
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.
Agreed, this is great!
denniwang
left a comment
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.
LGTM, looks like we need to write a lot more documentation though...
| routes = [{ method: 'GET', path: '/health', description: 'Health check' }]; | ||
| } | ||
|
|
||
| const description = options.description || `TODO: Add a description of the ${handlerName} lambda.`; |
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.
Good question, I think its ok to leave the TODO, hopefully either the dev or one of the reviewers will catch it.
nourshoreibah
left a comment
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.
One small change but otherwise LGTM!
| echo "No README changes to commit" | ||
| else | ||
| git commit -m "chore: regenerate lambda READMEs" | ||
| git push |
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.
One small comment here: Since this workflow pushes a commit, we need to make sure we don't get race conditions with any other pipelines that also push commits (the terraform readme one in our case), otherwise they could error if running at same time. Can you either make this pipeline wait for the tf one or the tf one wait for this? Thank you!
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.
ooo good catch ty I will make this one wait for the tf one!
nourshoreibah
left a comment
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.
Thank you for the changes! Great Work!
ℹ️ Issue
Closes #94
📝 Description
Write a short summary of what you added. Why is it important? Any member of C4C should be able to read this and understand your contribution -- not just your team members.
generate-readmecommand to the lambda CLI tool with a general template displaying setup, endpoint, and test information✔️ Verification
What steps did you take to verify your changes work? These should be clear enough for someone to be able to clone the branch and follow the steps themselves.
Tested by using CLI to make a

test-lambda, which generated this README(this was before the workflow was changed to trigger on workflow_run instead of pull_request) You can also see it in action in this PR! After I updated this branch from main, the GitHub actions bot generated a readme for the new
authlambda.🏕️ (Optional) Future Work / Notes
Did you notice anything ugly during the course of this ticket? Any bugs, design challenges, or unexpected behavior? Write it down so we can clean it up in a future ticket!