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 3.6] Move tests/scripts/check_names.py to the framework #9854

Open
wants to merge 3 commits into
base: mbedtls-3.6
Choose a base branch
from
Open

[Backport 3.6] Move tests/scripts/check_names.py to the framework #9854

wants to merge 3 commits into from
+6 −969

Conversation

valeriosetti
Copy link
Contributor

@valeriosetti valeriosetti commented Dec 13, 2024 •
edited by eleuzi01
Loading

Description

Resolves Mbed-TLS/mbedtls-framework#73 for the mbedtls-3.6 branch.

PR checklist

Please remove the segment/s on either side of the | symbol as appropriate, and add any relevant link/s to the end of the line.
If the provided content is part of the present PR remove the # symbol.

Notes for the submitter

Please refer to the contributing guidelines, especially the
checklist for PR contributors.

Help make review efficient:

  • Multiple simple commits
    • please structure your PR into a series of small commits, each of which does one thing
  • Avoid force-push
    • please do not force-push to update your PR - just add new commit(s)
  • See our Guidelines for Contributors for more details about the review process.

@valeriosetti
Move files out of Mbed TLS
9c3e09f 
The following files are moved to the framework repo (deleted here):

tests/scripts/check_names.py

Signed-off-by: Valerio Setti <valerio.setti@nordicsemi.no>
@valeriosetti valeriosetti mentioned this pull request Dec 13, 2024
Open
@valeriosetti valeriosetti changed the title Move tests/scripts/check_names.py to the framework [Backport 3.6] Move tests/scripts/check_names.py to the framework Dec 13, 2024
@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 size-xs Estimated task size: extra small (a few hours at most) priority-high High priority - will be reviewed soon labels Dec 13, 2024
@valeriosetti valeriosetti force-pushed the issue73-3.6 branch 2 times, most recently from b32b978 to 139bf4a Compare December 16, 2024 14:18
@valeriosetti valeriosetti removed the needs-ci Needs to pass CI tests label Dec 17, 2024
eleuzi01
eleuzi01 reviewed Dec 18, 2024
View reviewed changes
Copy link
Contributor

@eleuzi01 eleuzi01 left a comment

Choose a reason for hiding this comment

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

LGTM overall, there are mentions of check_names.py in tests/scripts/list_internal_identifiers.py and tests/scripts/list-identifiers.sh maybe it's worth changing those mentions to framework/scripts/check_names.py since this file is no longer in the same folder.

@valeriosetti
component-basic-checks: fix paths of files moved to framework
79dfcb4 
Signed-off-by: Valerio Setti <valerio.setti@nordicsemi.no>
@valeriosetti
framework: update reference
9f47b3b 
Signed-off-by: Valerio Setti <valerio.setti@nordicsemi.no>
@valeriosetti
Copy link
Contributor Author

LGTM overall, there are mentions of check_names.py in tests/scripts/list_internal_identifiers.py and tests/scripts/list-identifiers.sh maybe it's worth changing those mentions to framework/scripts/check_names.py since this file is no longer in the same folder.

Thanks for the suggestion. I saw those references, but since there was no path involved I thought it was not worth mentioning this change. However you are right: previously the script was in the same folder, but now it's no more the case, so it's better to be explicit.

eleuzi01
eleuzi01 approved these changes Dec 18, 2024
View reviewed changes
Copy link
Contributor

@eleuzi01 eleuzi01 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!

@eleuzi01 eleuzi01 mentioned this pull request Dec 18, 2024
Open
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eleuzi01 eleuzi01 eleuzi01 approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
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 priority-high High priority - will be reviewed soon size-xs Estimated task size: extra small (a few hours at most)
Projects
Roadmap pull requests (new board)
Status: In Review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@valeriosetti @eleuzi01