-
Notifications
You must be signed in to change notification settings - Fork 1
Add clippy and fmt #5
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
Conversation
Signed-off-by: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
Signed-off-by: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
c5d7950
to
d339be8
Compare
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
@@ -1,5 +1,7 @@ | |||
// Copyright 2023 Contributors to the Parsec project. | |||
// SPDX-License-Identifier: Apache-2.0 | |||
#![allow(clippy::missing_safety_doc)] |
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.
Is clippy using publicly visible unsafe functions ?
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.
Yes the parsec_provider_provider_init
here is dealing with C pointers as it will be at the FFI boundary.
ci.sh
Outdated
shift | ||
done | ||
|
||
if [ "$BUILD_AND_TEST" ]; then |
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.
This is tricky. It will work as is, but it can be prone to bugs.
We should either check that the variable is non zero len [[ -n $var ]]
or perform a string comparison.
Or we can use boolean variables using BUILD_AND_TEST=false
at line 32 and BUILD_AND_TEST=true
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.
Sorry for the delay. Yes, I will update it with a boolean.
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.
Good overall, two tibits only .
This patch adds a ci_script GitHub action and splits the jobs to run specific tests. Signed-off-by: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
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
Adds clippy and fmt to the CI along with changes needed to pass.
Signed-off-by: Gowtham Suresh Kumar gowtham.sureshkumar@arm.com