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

Add code style check #93

Open
wants to merge 5 commits into
base: development
Choose a base branch
from
Open

Add code style check #93

wants to merge 5 commits into from

Conversation

tom-daubney-arm
Copy link

@tom-daubney-arm tom-daubney-arm commented Dec 6, 2023 •
edited
Loading

Fixes #48

PR relies on changes to the following files which are brought in on Mbed TLS PR #8523 which is now merged:

  • scripts/generate_driver_wrappers.py
  • scripts/mbedtls_dev/build_tree.py
  • tests/scripts/check-generated-files.sh

@tom-daubney-arm tom-daubney-arm added enhancement New feature or request size-s Estimated task size: small (~2d) 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 labels Dec 6, 2023
@tom-daubney-arm
Copy link
Author

@ronald-cron-arm I have just realised that I have not yet got the all.sh component in place. I will do that this afternoon so I will remove the needs-review label for the time being.

@tom-daubney-arm tom-daubney-arm added needs-work and removed needs-review Every commit must be reviewed by at least two team members labels Dec 6, 2023
@ronald-cron-arm ronald-cron-arm linked an issue Dec 6, 2023 that may be closed by this pull request
Add code style check #48
Open
2 tasks
@tom-daubney-arm tom-daubney-arm added needs-review Every commit must be reviewed by at least two team members and removed needs-work labels Dec 6, 2023
@tom-daubney-arm
Copy link
Author

@ronald-cron-arm I have just realised that I have not yet got the all.sh component in place. I will do that this afternoon so I will remove the needs-review label for the time being.

Now done and ready for review.

ronald-cron-arm
ronald-cron-arm requested changes Dec 13, 2023
View reviewed changes
.uncrustify.cfg Outdated
@@ -0,0 +1,240 @@
# Configuration options for Uncrustify specifying the Mbed TLS code style.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer to copy that file from Mbed TLS instead of committing it into the development branch.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, can change that.

Copy link
Author

Choose a reason for hiding this comment

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

I will need to add a new function to the psa_crypto.py script to allow for copying files from the root directory in order to achieve this since currently only things from sub-directories are copied.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes sure you can add one

tests/all_sh_components.txt

component_check_code_style () {
msg "Check C code style"
./scripts/code_style.py
Copy link
Contributor

Choose a reason for hiding this comment

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

I've tried to run it and it seems we miss check-generated-files.sh.

Copy link
Author

Choose a reason for hiding this comment

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

Hmm, ok not sure why. Will investigate.

Copy link
Author

Choose a reason for hiding this comment

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

As discussed, I misunderstood the meaning of the initial comment. I have added check-generated-files.sh to be copied in psa_crypto.py.

@tom-daubney-arm
Introduce uncrustify config file
df8f135 
Signed-off-by: Thomas Daubney <[email protected]>
@tom-daubney-arm
Add code style script to list of scripts to be copied
dc142ec 
Signed-off-by: Thomas Daubney <[email protected]>
@tom-daubney-arm
Add all.sh component for code style checking
7dc6591 
Signed-off-by: Thomas Daubney <[email protected]>
@tom-daubney-arm
Add check-generated-files.sh to PSA copy script
98c0f06 
Signed-off-by: Thomas Daubney <[email protected]>
@tom-daubney-arm
Change the way uncrustify config is added to project
ce66409 
Commit removes the existing uncrustify config file
and adds it for copying in psa_crypto.py. This required
the creation of a new function in psa_crypto.py called
copy_from_root.

Signed-off-by: Thomas Daubney <[email protected]>
@tom-daubney-arm
Copy link
Author

@ronald-cron-arm @gabor-mezei-arm merge conflict is resolved so this is ready for review.

@tom-daubney-arm tom-daubney-arm removed the needs-reviewer This PR needs someone to pick it up for review label Dec 20, 2023
gabor-mezei-arm
gabor-mezei-arm approved these changes Dec 21, 2023
View reviewed changes
Copy link

@gabor-mezei-arm gabor-mezei-arm left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gabor-mezei-arm gabor-mezei-arm gabor-mezei-arm approved these changes

@ronald-cron-arm ronald-cron-arm Awaiting requested review from ronald-cron-arm

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
enhancement New feature or request needs-review Every commit must be reviewed by at least two team members priority-high High priority - will be reviewed soon size-s Estimated task size: small (~2d)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add code style check
3 participants
@tom-daubney-arm @ronald-cron-arm @gabor-mezei-arm