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.

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

[Baremetal] Harmonize static function compiler flags #2865

Merged

Conversation

artokin
Copy link

@artokin artokin commented Sep 25, 2019

Description

Warnings are treated as errors in Mbed TLS test. An error
"ssl_parse_client_hello_v2’ defined but not used" can occur in some
specific configurations and therefore tests will break.

Use similar flags for static function "ssl_parse_client_hello_v2" as
what is used when calling the function to prevent the compilation
warning/error.

Status

READY

hanno-becker
hanno-becker previously approved these changes Sep 25, 2019
Copy link

@hanno-becker hanno-becker left a comment

Choose a reason for hiding this comment

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

LGTM

@hanno-becker hanno-becker requested a review from mpg September 25, 2019 13:00
@hanno-becker hanno-becker added branch: baremetal bug needs-ci Needs to pass CI tests needs-review Every commit must be reviewed by at least two team members, labels Sep 25, 2019
Warnings are treated as errors in Mbed TLS test. An error
"ssl_parse_client_hello_v2’ defined but not used" can occur in some
specific configurations and therefore tests will break.

Use similar flags for static function "ssl_parse_client_hello_v2" as
what is used when calling the function to prevent the compilation
warning/error.
@artokin
Copy link
Author

artokin commented Sep 30, 2019

Force pushed to get CI re-triggered.

@artokin artokin requested a review from jarlamsa September 30, 2019 08:13
Copy link
Contributor

@jarlamsa jarlamsa left a comment

Choose a reason for hiding this comment

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

LGTM

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 for fixing that.

@mpg mpg added approved Design and code approved - may be waiting for CI or backports and removed needs-ci Needs to pass CI tests needs-review Every commit must be reviewed by at least two team members, labels Oct 1, 2019
@simonbutcher simonbutcher added needs-ci Needs to pass CI tests and removed approved Design and code approved - may be waiting for CI or backports labels Oct 4, 2019
@simonbutcher
Copy link
Contributor

CI jobs have failed for no obvious reasonable reason. Just in case it's a spurious fault, I've restarted the job.

Downgrading from 'ready to merge' to 'needs: CI'.

@mpg
Copy link
Contributor

mpg commented Oct 7, 2019

Latest CI failures were only due to the known-flaky "DTLS reconnect" test in ssl-opt.sh. Restarting the jobs.

@mpg
Copy link
Contributor

mpg commented Oct 7, 2019

The only failure in the CI is the API-ABI check on pr-head (passing on pr-merge) which as discussed in the daily is an artefact of how the checker works and the fact that another PR merged in the meantime changed the ABI. Therefore, ignoring it and marking the PR as "ready for merge" anyway.

@mpg mpg added approved Design and code approved - may be waiting for CI or backports and removed needs-ci Needs to pass CI tests labels Oct 7, 2019
@simonbutcher simonbutcher merged commit d198672 into Mbed-TLS:baremetal Oct 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Design and code approved - may be waiting for CI or backports bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants