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

Adding documentation to help develop ALI lambdas and some useful scripts #6256

Merged
merged 6 commits into from
Feb 4, 2025

Conversation

jeanschmidt
Copy link
Contributor

@jeanschmidt jeanschmidt commented Feb 4, 2025

Added a README.md on terraform-aws-github-runner/modules/runners/lambdas/runners/README.md with instruction on how to develop and troubleshoot lambdas development.

Copy link

vercel bot commented Feb 4, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Updated (UTC)
torchci ⬜️ Ignored (Inspect) Visit Preview Feb 4, 2025 4:58pm

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 4, 2025
@zxiiro
Copy link
Collaborator

zxiiro commented Feb 4, 2025

Looks good to me. I suggested fixes for a few typos. Thanks for documenting this!

jeanschmidt and others added 4 commits February 4, 2025 17:57
…ADME.md

Co-authored-by: Thanh Ha <thanh.ha@linuxfoundation.org>
…ADME.md

Co-authored-by: Thanh Ha <thanh.ha@linuxfoundation.org>
…ADME.md

Co-authored-by: Thanh Ha <thanh.ha@linuxfoundation.org>
…ADME.md

Co-authored-by: Thanh Ha <thanh.ha@linuxfoundation.org>
@jeanschmidt jeanschmidt merged commit 88e4f1e into main Feb 4, 2025
6 checks passed
@jeanschmidt jeanschmidt deleted the jeanschmidt/document_lambda_dev branch February 4, 2025 17:13
Copy link
Contributor

@ZainRizvi ZainRizvi left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together, this doc will be really helpful as people onboard


### Expectations

It is **required** to submit code:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
It is **required** to submit code:
**Requirements** when submitting code:

Comment on lines +39 to +41
* No linting warnings are being thrown;
* The code must be fully compilable to javascript;
* Code formatting are according to current standards (prettier);
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't CI fail if these requirements aren't met?

Can we instead change these three lines to: "All CI checks pass"


It is *advisable* to strive to when submitting code:

* Improve as much as possible unit test code coverage;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Improve as much as possible unit test code coverage;
* Improve unit test code coverage as much as possible;


### Makefile helpers

Those are primarly used for CI, but, it might be useful to understand, there are 3 commands:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Those are primarly used for CI, but, it might be useful to understand, there are 3 commands:
Those are primarly used for CI, but, it might be useful to understand. There are 3 commands:


**WARNING: In practice, even with canary, we only have production environment, be aware that you can break things when running tests!**

So, it is not really recommended to do so, unless troubleshooting something that you have limited understanding and can't replicate locally.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
So, it is not really recommended to do so, unless troubleshooting something that you have limited understanding and can't replicate locally.
So, it is not really recommended to do so, unless troubleshooting something that you have a limited understanding of and can't replicate locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants