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

Fix for filename capture not working #7050

Merged
merged 3 commits into from Jun 4, 2018

Conversation

Projects
None yet
6 participants
@SenRamakri
Contributor

SenRamakri commented May 29, 2018

Description

This is the fix for #7025
There was a bug in the code where in filename capture wont work properly and also we should disable filename capture release builds. This change fixes that.

Pull request type

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

@SenRamakri SenRamakri requested review from kegilbert, geky and 0xc0170 May 29, 2018

@SenRamakri

This comment has been minimized.

Contributor

SenRamakri commented May 29, 2018

This is a bug fix and we should try to get this in for 5.9. Is this something which can go into 5.9 RC2?

@0xc0170

This comment has been minimized.

Member

0xc0170 commented May 30, 2018

If this is found during oob then in the next release candidate, otherwise the next patch release.

I assume this goes to the next patch release.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented May 30, 2018

Can be the first commit extended ?one liner for those changes do not look sufficient (how is it fixing the bug?)

@SenRamakri

This comment has been minimized.

Contributor

SenRamakri commented May 30, 2018

@0xc0170 - Please see my change to function handle_error for the fix.

@cmonr

Code changes look good to me, but echoing @0xc0170 sentiment about the code changes vs commit message.

Either more detail should be added to that first commit message, or it should be split into two, since that single commit also contains the NDEBUG additions.

@SenRamakri

This comment has been minimized.

Contributor

SenRamakri commented May 31, 2018

@0xc0170 @cmonr - Good point, I'll split them into separate commits and re-push.

@SenRamakri SenRamakri force-pushed the SenRamakri:sen_ErrorHandlingFilenameFix branch from 23fa1bc to 576bd61 May 31, 2018

@SenRamakri

This comment has been minimized.

Contributor

SenRamakri commented May 31, 2018

@cmonr @0xc0170 - I have split the commits as you suggested plus one more change to fix error report generation when calling error API, please review.

@cmonr

cmonr approved these changes May 31, 2018

LGTM

@cmonr cmonr added needs: CI and removed needs: review labels May 31, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented May 31, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Jun 1, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci

This comment has been minimized.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jun 1, 2018

/morph uvisor-test

@mbed-ci

This comment has been minimized.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jun 1, 2018

/morph export-build

@mbed-ci

This comment has been minimized.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jun 3, 2018

/morph uvisor-test

@0xc0170

0xc0170 approved these changes Jun 4, 2018

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jun 4, 2018

/morph uvisor-test

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jun 4, 2018

@orenc17 Please help to restart this job

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

@cmonr cmonr merged commit 50cd664 into ARMmbed:master Jun 4, 2018

13 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, 919 files
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 9316 cycles (-266 cycles)
Details
travis-ci/gitattributestest Local gitattributestest 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

@cmonr cmonr removed the ready for merge label Jun 7, 2018

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