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.

Sign up for GitHub

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

Jump to bottom

Backport: Clean up & improve PK write test functions #7456

Merged
merged 4 commits into from
Apr 24, 2023
Merged

Backport: Clean up & improve PK write test functions #7456

merged 4 commits into from
Apr 24, 2023

Conversation

valeriosetti
Copy link
Contributor

@valeriosetti valeriosetti commented Apr 18, 2023 •
edited by mpg
Loading

This is the backport of issue #7446 as solved in PR #7449.

Gatekeeper checklist

  • changelog not required
  • backport not required: this is the backport
  • tests not required

@valeriosetti valeriosetti added needs-review Every commit must be reviewed by at least two team members, needs-ci Needs to pass CI tests needs-reviewer This PR needs someone to pick it up for review labels Apr 18, 2023
@valeriosetti valeriosetti requested a review from mpg April 18, 2023 15:02
@valeriosetti valeriosetti self-assigned this Apr 18, 2023
@valeriosetti valeriosetti mentioned this pull request Apr 18, 2023
Merged
3 tasks
mpg
mpg approved these changes Apr 19, 2023
View reviewed changes
Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

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

LGTM

@mpg
Copy link
Contributor

mpg commented Apr 19, 2023

Note for the future: as a reviewer, I like it when backports have the same commit structure as the original PR, because then I can use git range-diff to check it's just the same changes. (Also, as a backport author, that's easily achieved by using git cherry-pick development..original_branch.) (Here I manually made a diff of diffs, but I don't know how to make it as readable as the output from git range-diff.)

@valeriosetti valeriosetti removed the needs-ci Needs to pass CI tests label Apr 19, 2023
AndrzejKurek
AndrzejKurek previously approved these changes Apr 20, 2023
View reviewed changes
@AndrzejKurek AndrzejKurek added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review labels Apr 20, 2023
AndrzejKurek
AndrzejKurek reviewed Apr 20, 2023
View reviewed changes
tests/data_files/Makefile Outdated
@@ -888,6 +888,57 @@ ec_prv.pk8param.pem: ec_prv.pk8param.der
$(OPENSSL) pkey -in $< -inform DER -out $@
all_final += ec_prv.pk8param.pem

################################################################
#### Convert PEM keys in DER format
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
#### Convert PEM keys in DER format
#### Convert PEM keys to DER format

Same suggestion as in the original PR.

@daverodgman daverodgman added the priority-medium Medium priority - this can be reviewed as time permits label Apr 21, 2023
mpg
mpg requested changes Apr 24, 2023
View reviewed changes
Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

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

This needs updating to match changes in the original PR.

@mpg mpg added needs-work and removed approved Design and code approved - may be waiting for CI or backports labels Apr 24, 2023
mpg
mpg approved these changes Apr 24, 2023
View reviewed changes
Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@mpg mpg added needs-review Every commit must be reviewed by at least two team members, and removed needs-work labels Apr 24, 2023
@mpg mpg added the needs-ci Needs to pass CI tests label Apr 24, 2023
@mpg mpg requested a review from AndrzejKurek April 24, 2023 09:22
AndrzejKurek
AndrzejKurek approved these changes Apr 24, 2023
View reviewed changes
Copy link
Contributor

@AndrzejKurek AndrzejKurek left a comment

Choose a reason for hiding this comment

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

Faithful backport.

@AndrzejKurek AndrzejKurek added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, labels Apr 24, 2023
@mpg mpg merged commit c14776c into Mbed-TLS:mbedtls-2.28 Apr 24, 2023
@valeriosetti valeriosetti deleted the issue7446-backport branch December 6, 2024 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndrzejKurek AndrzejKurek AndrzejKurek approved these changes

@mpg mpg mpg approved these changes

Assignees

@valeriosetti valeriosetti

Labels
approved Design and code approved - may be waiting for CI or backports needs-ci Needs to pass CI tests priority-medium Medium priority - this can be reviewed as time permits
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@valeriosetti @mpg @AndrzejKurek @daverodgman