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

Remove semicolon at the end of #define #7355

Merged
merged 1 commit into from Jun 29, 2018

Conversation

Projects
None yet
5 participants
@deepikabhavnani
Contributor

deepikabhavnani commented Jun 27, 2018

Description

Remove semicolon followed by pre-processor defines. Noticed build failures when building code with "EVR_RTX_DISABLE" set.

Pull request type

[X] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

Note: Fix in CMSIS repo is present.

@cmonr cmonr requested a review from bulislaw Jun 27, 2018

@cmonr cmonr added the needs: review label Jun 27, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jun 27, 2018

Which code was erroring/failing before this fix?

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jun 28, 2018

Note: Fix in CMSIS repo is present.

Worth referring to the specific SHA/pull request. I can't locate it on develop branch there. Seems still wrong to me.

This should be fixed first upstream : https://github.com/ARM-software/CMSIS_5/blame/develop/CMSIS/RTOS2/RTX/Include/rtx_evr.h#L836 ?

@JonatanAntoni

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jun 28, 2018

I sent upstream fix as I found these extra ; there (compare to the rest of the file, should not be there)

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jun 28, 2018

Upstream fixed (It was merged)

@0xc0170

As now upstream is fixed, this will be in when we update

@bulislaw

In the future please reference the upstream commit in the commit description.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jun 28, 2018

/morph build

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Jun 28, 2018

@mbed-ci

This comment has been minimized.

mbed-ci commented Jun 28, 2018

Build : SUCCESS

Build number : 2469
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7355/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci

This comment has been minimized.

@deepikabhavnani

This comment has been minimized.

Contributor

deepikabhavnani commented Jun 28, 2018

In the future please reference the upstream commit in the commit description.

I left the note in description "Note: Fix in CMSIS repo is present..." Not sure why grep on my local repo didn't show me semicolons in upstream (may be old copy / wrong command). Lesson learned will always check the web GitHub repo instead local repo.

@0xc0170 - Thanks for adding PR to CMSIS.

@deepikabhavnani

This comment has been minimized.

Contributor

deepikabhavnani commented Jun 28, 2018

Which code was erroring/failing before this fix?

EventRecorder code, when disabling RTX specific events

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jun 28, 2018

Pipe closed.

/morph export-build

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jun 29, 2018

/morph export-build

@mbed-ci

This comment has been minimized.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jun 29, 2018

ಠ_ಠ

/morph export-build

@mbed-ci

This comment has been minimized.

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Jun 29, 2018

@0xc0170 0xc0170 merged commit 2be3c13 into ARMmbed:master Jun 29, 2018

14 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/astyle Passed, 929 files
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 10227 cycles (+242 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 9964B (+0.00%)
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details
@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jun 29, 2018

I added the upstream fix to the merge commit msg

@deepikabhavnani deepikabhavnani deleted the deepikabhavnani:rtx_header_fix branch Jun 29, 2018

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