improve buildspec for AL2023 kTLS #4989
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Removed redundant `yum upgrade -y` command, keeping only `yum update -y`. - Ensured the installation of a specific version of Nix for consistency. - Improved comments for better readability and maintenance. - Verified that the TLS kernel module loads correctly. - Unified command formatting for better readability.
|
Thank you for your PR! We'll take a look and review this change. |
jouho
reviewed
Jan 8, 2025
| @@ -8,21 +8,21 @@ env: | |||
| phases: | |||
| install: | |||
| commands: | |||
| - yum update -y; yum upgrade -y | |||
| - yum update -y | |||
There was a problem hiding this comment.
yum upgrade removes the outdated packages on top of upgrading packages to the latest version. Why not use yum upgrade instead?
There was a problem hiding this comment.
Hey, great question, it would be feasible:
- To avoid unintended package removals in a controlled build environment
- To maintain more predictable package states
- To reduce build time by eliminating a potentially redundant step
There was a problem hiding this comment.
Is there a reason we are using both upgrade and update? @dougch
If safe to do so, my suggestion would be:
Suggested change
| - yum update -y | |
| - yum upgrade -y |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changelog:
Removed redundant yum upgrade -y command, retaining only yum update -y.
Ensured consistent Nix installation by specifying a version.
Enhanced comments for improved readability and maintenance.
Confirmed correct loading of the TLS kernel module.
Standardized command formatting for better readability.
Release Summary:
Resolved issues:
Description of changes:
Describe s2n’s current behavior and how your code changes that behavior. If there are no issues this PR is resolving, explain why this change is necessary.
Call-outs:
Address any potentially confusing code. Is there code added that needs to be cleaned up later? Is there code that is missing because it’s still in development? If a callout is specific to a section of code, it might make more sense to leave a comment on your own PR file diff.
Testing:
Describe the testing methods used (unit tests, fuzz tests, etc.).
Explain any manual testing performed.
Provide verification steps for the reviewer.
Assure reviewers that the PR is safe and effective.
If it's a refactor change, demonstrate that the intended behavior hasn't changed.
Remember:
Include unit tests for any change to the library source code.
Include CBMC proofs for changes to core stuffer or blob methods.
For changes to the CI or tests:
Ensure the test succeeds for valid input.
Ensure the test fails for invalid input (e.g., a test for memory leaks fails when a memory leak is committed).
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.