-
Notifications
You must be signed in to change notification settings - Fork 48
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
Switch to GitHub Actions for CI/CD #361
Conversation
2095f24
to
cb2c499
Compare
cd8515b
to
6a87727
Compare
options: --ulimit core=-1 --ulimit memlock=-1:-1 | ||
steps: | ||
- name: Checkout the repository | ||
uses: actions/checkout@5a4ac9002d0be2fb38bd78e4b4dbde5606d7042f |
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.
While pinning actions via the commit SHA (in lieu of tags) is good practice from a security/hardening standpoint, as it guarantees cross-build consistency, it means we're on the hook for ensuring we don't miss out on potentially critical security updates. What's the plan for ensuring we remain up-to-date on future updates for these actions?
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'm pretty sure dependabot can help us with these! We will have to specifically set up a dependabot.yml
, but there is a github-actions
package-ecosystem value.
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.
Inline comments/suggestions aside, I'd like to see the majority of the ~70 commits squashed prior to merge.
csharp/AppEncryption/AppEncryption.Tests/AppEncryption/Persistence/DynamoDbMetastoreImplTest.cs
Outdated
Show resolved
Hide resolved
csharp/AppEncryption/AppEncryption.Tests/AppEncryption/Persistence/DynamoDbMetastoreImplTest.cs
Outdated
Show resolved
Hide resolved
csharp/AppEncryption/AppEncryption.Tests/AppEncryption/Persistence/DynamoDbMetastoreImplTest.cs
Outdated
Show resolved
Hide resolved
csharp/AppEncryption/AppEncryption.Tests/AppEncryption/SessionFactoryTest.cs
Show resolved
Hide resolved
csharp/AppEncryption/AppEncryption.Tests/AppEncryption/SessionFactoryTest.cs
Show resolved
Hide resolved
ba06f23
to
10d2ede
Compare
c306caa
to
2446ee1
Compare
bdc2c94
to
7313a0f
Compare
This PR also skips running 2 integration tests that fail intermittently in the CI pipeline. |
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
Necessary parts:
CI components
CD components