-
Notifications
You must be signed in to change notification settings - Fork 1
Fix bug where unable to change default on enum properties #71
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
Conversation
…he value had been converted to an index into the enum
|
I'm a little unsure about the itemtype naming fix - it seems to build, but I'm not 100% of the wider implications that this may have. |
|
okay, my uncertainty was justified, the test build fails with the itemtypes not found, so there's another point where this needs to be adapted. |
|
looking at the test module: auto& colorType = Root::Colors::itemtype;
auto& quotientType = Root::Quotients::itemtype;
auto& smallMapType = Root::SmallMap::itemtype;
auto& numberMapType = Root::NumberMap::itemtype;those itemtypes are now called colorItemType, quotientsItemType etc - not sure if that's actually a good way to address the issue. |
|
I am a bit unsure now, with my changes, my firware builds without issues, only this explicit access to itemType in the test fails - not sure what is the relevance. |
|
Thanks for this fix. Can you restrict this to just the 'defaults and enums' and we'll look at the itemtype naming separately. |
|
new push with just 0cfd545 |
|
Thanks. Amended test schema to ensure this gets covered. |
|
Thanks for fixing this bug! Note for next time: Remember to create a new branch e.g. |
fixed a bug in dbgen.py, where a default value was checked against an enum after the value had been converted to an index into the enum as documented in #70
I feel that the 2nd check is redundant as the default has already been verified against the enum and the index was successfully calulated, therefore I believe it can be just removed.