Skip to content

Remove unused arguments from detect targets #3439

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

Merged
merged 1 commit into from
Dec 21, 2016

Conversation

theotherjimmy
Copy link
Contributor

Status

READY

Todos

  • Tests - Manual verification that mbed detect still works and that mbed detect -f works

Resolves #1996

Copy link
Contributor

@bridadan bridadan left a comment

Choose a reason for hiding this comment

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

Few comments below

@@ -68,14 +70,17 @@ def main():
# parameters like 'toolchains_filter' are also set.
muts = get_autodetected_MUTS_list()

mcu_filter = options.general_filter_regex or [".*"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Any reason why the default value for the --filter argument can't be ".*"?

It seems kind of odd to accept an array of regex expressions. Wouldn't you just change your regex to be more accommodating?

Also, the current ArgParse option will set options.general_filter_regex to just a string, not an array. Won't this blow up below?

@bridadan
Copy link
Contributor

As far as I know, none of the CI scripts hit this script, but I'll run the basic one just in case.

/morph test

@mbed-bot
Copy link

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1285

All builds and test passed!

@bridadan
Copy link
Contributor

Ready pending review from @adbridge

@adbridge adbridge requested review from adbridge and removed request for adbridge December 19, 2016 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants