Skip to content
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

Add some type checking of configs #4000

Merged
merged 1 commit into from
Mar 29, 2017

Conversation

theotherjimmy
Copy link
Contributor

@theotherjimmy theotherjimmy commented Mar 22, 2017

Before, the types were not checked and just expected. The old behavior
would cause lots of tracebacks, or, much worse, convert things like:

{
     "target_overrides": {
        "*": {
		"target.macros_add": "CONFIG_GPIO_AS_PINRESET"
        }
     }
}

into a definition of each of the letters as macros that expand to
nothing, causing massive compilation problems.

I resolved this by adding some type checking to the config data. Now
there is a type check for most of the fields within a library and
application configurations.

resolves #3985

Testing

  • /morph test

Before, the types were not checked and just expected. The old behavior
would cause lots of tracebacks, or, much worse, convert things like:
```
{
     "target_overrides": {
        "*": {
		"target.macros_add": "CONFIG_GPIO_AS_PINRESET"
        }
     }
}
```
into a definition of each of the letters as macros that expand to
nothing, causing massive compilation problems.

I resolved this by adding some type checking to the config data. Now
there is a type check for most of the fields within a library and
application configurations.
@theotherjimmy
Copy link
Contributor Author

@jaustin Be advised. More weird type-related fixes welcome.
@bridadan is that a valid test?

@theotherjimmy
Copy link
Contributor Author

I got PR 4000 🎉 🎈 🍰

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 23, 2017

Add some type checking of configs #4000

🍾

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 23, 2017

@bridadan is that a valid test?

tools/tests/config or config_test

@bridadan
Copy link
Contributor

@theotherjimmy Please just use the regular morph test, it includes the example build step already.

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.

This looks great!

@adbridge
Copy link
Contributor

/morph test

@mbed-bot
Copy link

Result: SUCCESS

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

/morph test

Output

mbed Build Number: 1728

All builds and test passed!

@sg- sg- merged commit 405ab0c into ARMmbed:master Mar 29, 2017
@theotherjimmy theotherjimmy deleted the config-typechecking branch March 29, 2017 21:42
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.

config: macros_remove doesn't remove CONFIG_GPIO_AS_PINRESET macro for NRF52_DK
6 participants