Skip to content
This repository has been archived by the owner on Jul 12, 2020. It is now read-only.

Adds GetEnumItems #119

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

Kampfkarren
Copy link
Contributor

I'm not sure where I'd add this to FEATURES.md.

@Kampfkarren
Copy link
Contributor Author

Didn't notice this failed. Fixing.

@coveralls
Copy link

coveralls commented Aug 22, 2018

Coverage Status

Coverage increased (+0.03%) to 97.4% when pulling bb344b1 on Kampfkarren:datastores-4 into 5d47a47 on LPGhatguy:master.


local enumItemFunny = enumItems[2]
assert.equal(enumItemFunny.Name, "Funny")
assert.equal(enumItemFunny.Value, 2)
Copy link
Owner

Choose a reason for hiding this comment

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

This test is flaky and depends on iteration order. If we're going to keep the current implementation, the order needs to either be unimportant or deterministic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll fix it.

return function()
return enumItems
end
end
Copy link
Owner

Choose a reason for hiding this comment

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

This is kind of a bolt-on solution, we should probably make a table containing the enum variant's methods and explicitly index into it instead of comparing the key with a constant.

Additionally, we should return the same function every time, otherwise Foo.GetEnumItems ~= Foo.GetEnumItems!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll fix it. [2]

Copy link
Contributor Author

@Kampfkarren Kampfkarren Aug 23, 2018

Choose a reason for hiding this comment

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

So about that...
image
I'm not sure if you want to replicate this strange behavior or not.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants