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

Netbeans Exporter Bugfix #5901

Merged
merged 2 commits into from Jan 29, 2018

Conversation

Projects
None yet
5 participants
@cmens23

cmens23 commented Jan 23, 2018

Description

Fixed a bug on multiple Sub-Directories with same name.
Was not discovered correctly before.

Problem occured on path with same name like:
storage/filesystem/littlefs/littlefs

Status

READY

Steps to test or reproduce

mbed export -i netbeans

@mbed-ci

This comment has been minimized.

mbed-ci commented Jan 23, 2018

Automatic CI verification build not done, please verify manually.

@0xc0170 0xc0170 requested a review from theotherjimmy Jan 23, 2018

matched.append(element)
# Compare the Element in Previous Dir with the Elements in Current Dir
# and add the Differences to the match-List
if len(dir_list) <= len(prev_dir_list):

This comment has been minimized.

@theotherjimmy

theotherjimmy Jan 23, 2018

Contributor

I prefer this implemented as:

for prev_dir, cur_dir in zip(prev_dir_list, dir_list):
    if cur_dir == prev_dir:
        ...

With that, you don't need the if statement. zip will terminate when either list ends; docs.

This comment has been minimized.

@cmens23

cmens23 Jan 24, 2018

Sorry for that, didn't know that this can be done much easier! Thank you!

This comment has been minimized.

@theotherjimmy

theotherjimmy Jan 24, 2018

Contributor

No need to apologize; that's what review is for.

Clemens Mandl
# and add the equal Elements to the match-List
for elem_prev_dir, elem_cur_dir in zip(prev_dir_list, dir_list):
if elem_prev_dir == elem_cur_dir:
matched.append(elem_cur_dir)

This comment has been minimized.

@theotherjimmy

theotherjimmy Jan 24, 2018

Contributor

The use of elem in these variable names dose not add any meaning for me. I would like to seem them removed, but I won't block this PR for it.

This comment has been minimized.

@cmens23

cmens23 Jan 25, 2018

If I remove the prefix elem it will not work as expected, because the variables prev_dir and cur_dir are already defined in scope.

This comment has been minimized.

@theotherjimmy

theotherjimmy Jan 25, 2018

Contributor

That's part of why I would like another name: the current name is almost a name clash. Like I said, not blocking just a wish. Let me know if you come up with something.

@cmonr cmonr added needs: CI and removed needs: review labels Jan 25, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jan 25, 2018

/morph build

1 similar comment
@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jan 26, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Jan 26, 2018

Build : SUCCESS

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

Triggering tests

/morph test
/morph uvisor-test
/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 Jan 26, 2018

Holding off on reissuing test. uVisor build CI has been experiencing an odd issue across multiple PRs today.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jan 29, 2018

/morph uvisor-test

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

@cmonr cmonr merged commit cf5065c into ARMmbed:master Jan 29, 2018

19 checks passed

ARM mbed CI Verification build successful.
Details
AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter 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/docs Local docs testing has passed
Details
travis-ci/events Local events testing has passed
Details
travis-ci/littlefs Local littlefs testing has passed
Details
travis-ci/mbed2-ATMEL Local mbed2-ATMEL testing has passed
Details
travis-ci/mbed2-MAXIM Local mbed2-MAXIM testing has passed
Details
travis-ci/mbed2-NORDIC Local mbed2-NORDIC testing has passed
Details
travis-ci/mbed2-NUVOTON Local mbed2-NUVOTON testing has passed
Details
travis-ci/mbed2-NXP Local mbed2-NXP testing has passed
Details
travis-ci/mbed2-RENESAS Local mbed2-RENESAS testing has passed
Details
travis-ci/mbed2-SILICON_LABS Local mbed2-SILICON_LABS testing has passed
Details
travis-ci/mbed2-STM Local mbed2-STM testing has passed
Details
travis-ci/tools Local tools testing has passed
Details

@cmonr cmonr removed the ready for merge label Jan 29, 2018

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