-
Notifications
You must be signed in to change notification settings - Fork 139
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
Move computesha256 to typescript #546
Move computesha256 to typescript #546
Conversation
- Port the computesha256 from bash to typescript. Signed-off-by: naveensrinivasan <[email protected]>
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 @naveensrinivasan
Thanks for providing the link to the template you use, I'll take a look to learn a little more too
- Updated to use the native crypto library - Included README instructions - Some minor nits. Signed-off-by: naveensrinivasan <[email protected]>
Signed-off-by: naveensrinivasan <[email protected]>
4eb8c5b
to
b9992ad
Compare
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.
Did a quick pass and this looks pretty good.
Signed-off-by: naveensrinivasan <[email protected]>
- Included the workflow to validate the computesha256 github action Signed-off-by: naveensrinivasan <[email protected]>
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.
Please bear with me trying to reproduce. I suspect we have different versions of the tool installed.
Added a few comments
- Included permissions for the workflow - Renamed the workflow file - Updated the input variable to `path` - Changed the `run` function to non-async - Updated the README instructions - Included the tag for pinned SHA
1ac1ea4
to
8bb45a9
Compare
Signed-off-by: naveensrinivasan <[email protected]>
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
NOTE: We need to mark the pre-submit as required after merge. |
- Moved the action within the pre-submit.actions.yml - Removed the redundant instructions in README Signed-off-by: naveensrinivasan <[email protected]>
@ianlewis @laurentsimon Thank you for all the review and feedback! Excited to have this merged! |
Great work @naveensrinivasan. The PR looks great! |
Signed-off-by: naveensrinivasan [email protected]