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 Lua analyzer #1057

Merged
merged 1 commit into from
Apr 15, 2016
Merged

add Lua analyzer #1057

merged 1 commit into from
Apr 15, 2016

Conversation

3van
Copy link
Contributor

@3van 3van commented Feb 19, 2016

Based off of the Lua 5.3 Reference Manual.
I ran it against our (crazy, massively large) codebase and didn't see it trip up on any of our Lua.
I signed the OCA forever ago when I used to contribute to the ndbcluster engine on MySQL; not sure if I need to sign anything else for this, but let me know if I do.

comments are fun
@vladak
Copy link
Member

vladak commented Feb 19, 2016

Nice ! would it be possible to add some tests ?

@3van
Copy link
Contributor Author

3van commented Feb 19, 2016

Sure. Let me see if I can whip up a representative Lua sample file.

*/

/*
* Get Golang symbols - ignores comments, strings, keywords
Copy link
Contributor

Choose a reason for hiding this comment

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

get LUA sumbols? ;)
copy paste ? :-D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hah! I thought I grepped for all those. Guess not. 💃

Copy link
Contributor

Choose a reason for hiding this comment

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

grep? pff ... you should have used grok to find all of them! :-D

@tarzanek tarzanek added this to the 0.13 milestone Feb 22, 2016
@tarzanek
Copy link
Contributor

tarzanek commented Mar 8, 2016

@3van any news?
btw. I don't have a problem to integrate (don't want to hold this up) even without tests, if you promise you will add them in next pull req
so if you will fix the typo, I'll happily merge (and I am still figuring out if your mysql OCA can be used, but I hope it can)

@tarzanek
Copy link
Contributor

tarzanek commented Mar 8, 2016

@3van confirming your OCA, so basically we're just waiting for you :)
tia
L

@3van
Copy link
Contributor Author

3van commented Mar 11, 2016

I haven't had time to work on this recently but I'll at least fix the typo and see if I can get some basic tests done this weekend. Thanks for verifying the OCA; that's good to know as well.

@tarzanek
Copy link
Contributor

@3van any news? :) I'd like to merge this, so we can move on, we can push tests with new pull req (divide and conquer)

@tarzanek
Copy link
Contributor

@3van I am just going to accept this, let's move the tests to a new pull req, I will fix the silly typos for you

@tarzanek
Copy link
Contributor

and thank you @3van ! :)

@tarzanek tarzanek merged commit 0f5a728 into oracle:master Apr 15, 2016
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.

None yet

3 participants