Skip to content

Supported lists #460

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 2 commits into from
Sep 15, 2017
Merged

Supported lists #460

merged 2 commits into from
Sep 15, 2017

Conversation

mbartling
Copy link
Contributor

Added ability to list toolchains, ides, targets in addition to the supported matrix output. Solves #306.

NOTE: This depends on ARMmbed/mbed-os#3993. @screamerbg @theotherjimmy can you guys think of a workaround? Should I just add a try catch?

   if supported:
        popen(['python', '-u', os.path.join(tools_dir, 'project.py')]
# The offending line
              + (['-S', supported] if supported else []) + (['-v'] if very_verbose else []),
              env=env)
        return

@bridadan
Copy link
Contributor

bridadan commented Apr 4, 2017

Might be worth noting that this will probably fail with an error with mbed OS releases that didn't have the capability added in ARMmbed/mbed-os#3993. Should we be handling this error in this case (might not be necessary)?

@mbartling
Copy link
Contributor Author

I'm not sure, @screamerbg what do you think?

@screamerbg
Copy link
Contributor

@mbartling @bridadan I agree about backwards compatibility issue. This PR should follow after ARMmbed/mbed-os#3993 becomes mainline and widely used.

@mbartling
Copy link
Contributor Author

FYI ARMmbed/mbed-os#3993 is now merged

@screamerbg
Copy link
Contributor

screamerbg commented Apr 13, 2017

@mbartling Can you rebase please?

@screamerbg
Copy link
Contributor

@mbartling Do you feel this is ready to go in?
cc @theotherjimmy

@mbartling
Copy link
Contributor Author

It's been a few months, do you think this has been main streamed in mbed-os?

@mbartling
Copy link
Contributor Author

I think this should be ready to merge

@screamerbg
Copy link
Contributor

@theotherjimmy please review.
@mbartling please rebase.

@theotherjimmy
Copy link
Contributor

@screamerbg After the rebase, I'll review. @mbartling Could you ping me after the rebase is done so that I can review?

@mbartling
Copy link
Contributor Author

@theotherjimmy rebase is complete

Copy link
Contributor

@theotherjimmy theotherjimmy left a comment

Choose a reason for hiding this comment

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

Looks great!

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.

4 participants