Skip to content

Conversation

@rpavlik
Copy link
Member

@rpavlik rpavlik commented Sep 21, 2015

These were pulled out of the massive gesture pull request and subsequent modifications. They seem to be reasonable now, still need unit tests for the registered string map. @mars979 ?

gfrolov and others added 6 commits September 21, 2015 16:21
No more overriding operator& or providing implicit conversion.
It's also now properly in a namespace, and uses traits classes
to determine the underlying value type as well as the "invalid/empty"
sentinel value.
Includes some basic tests.
Some modifications on rebase.
@gfrolov
Copy link
Contributor

gfrolov commented Sep 21, 2015

Added the unit tests on registered string map and correlated map as well. Tests are passing

Copy link
Member Author

Choose a reason for hiding this comment

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

Can just use constructor, safer and easier.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, I'd move this fixture down to just before the tests that use it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, using constructor now and moved fixture down before tests.

@rpavlik
Copy link
Member Author

rpavlik commented Sep 22, 2015

Can merge once the few changes are done.

…tUp function, remove unused lines, use _eq not _streq for c++ str
@gfrolov
Copy link
Contributor

gfrolov commented Sep 22, 2015

Modified the tests according to comments above, ready for merge now

gfrolov pushed a commit that referenced this pull request Sep 22, 2015
@gfrolov gfrolov merged commit 0305b22 into master Sep 22, 2015
@rpavlik rpavlik deleted the gesture-data-structures branch September 22, 2015 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants