Skip to content

Add IgnoreSuffix feature #93

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.

Sign up for GitHub

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

Jump to bottom
Merged
wu-sheng merged 12 commits into from
Jan 13, 2022
Merged

Add IgnoreSuffix feature #93

wu-sheng merged 12 commits into from
Jan 13, 2022

Conversation

@alonelaval
Copy link
Member

@alonelaval alonelaval commented Jan 10, 2022

No description provided.

@wu-sheng wu-sheng added this to the 0.7.0 milestone Jan 10, 2022
@wu-sheng wu-sheng added the enhancement New feature or request label Jan 10, 2022
wu-sheng
wu-sheng reviewed Jan 11, 2022
View reviewed changes
wu-sheng
wu-sheng reviewed Jan 11, 2022
View reviewed changes
dmsolr
dmsolr reviewed Jan 11, 2022
View reviewed changes
wu-sheng
wu-sheng reviewed Jan 11, 2022
View reviewed changes
wu-sheng
wu-sheng reviewed Jan 11, 2022
View reviewed changes
wu-sheng
wu-sheng reviewed Jan 11, 2022
View reviewed changes
wu-sheng
wu-sheng reviewed Jan 11, 2022
View reviewed changes
@wu-sheng
Copy link
Member

wu-sheng commented Jan 12, 2022

@alonelaval CI fails

https://github.com/apache/skywalking-nginx-lua/runs/4773611968?check_suite_focus=true

@alonelaval
Copy link
Member Author

alonelaval commented Jan 12, 2022

https://github.com/apache/skywalking-nginx-lua/runs/4773611968?check_suite_focus=true

OK，I will fix.

@tzssangglass
Copy link
Member

tzssangglass commented Jan 12, 2022

hi, @alonelaval the CI installation of OpenResty includes luajit, try to use /usr/local/openresty-debug/luajit/bin/luajit instead of lua to test, ffi is a module of luajit.

@alonelaval
Copy link
Member Author

alonelaval commented Jan 12, 2022

hi, @alonelaval the CI installation of OpenResty includes luajit, try to use /usr/local/openresty-debug/luajit/bin/luajit instead of lua to test, ffi is a module of luajit.

no work，in my local why luarocks installed package 'luaunit' not found.

@wu-sheng wu-sheng mentioned this pull request Jan 13, 2022
Merged
wu-sheng
wu-sheng reviewed Jan 13, 2022
View reviewed changes
wu-sheng
wu-sheng reviewed Jan 13, 2022
View reviewed changes
wu-sheng
wu-sheng reviewed Jan 13, 2022
View reviewed changes
wu-sheng
wu-sheng reviewed Jan 13, 2022
View reviewed changes
@wu-sheng wu-sheng requested a review from dmsolr January 13, 2022 07:59
@wu-sheng
Copy link
Member

wu-sheng commented Jan 13, 2022

@membphis @dmsolr @tzssangglass Could you review the PR again?

membphis
membphis approved these changes Jan 13, 2022
View reviewed changes
Copy link
Member

@membphis membphis left a comment

Choose a reason for hiding this comment

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

LGTM

wu-sheng
wu-sheng approved these changes Jan 13, 2022
View reviewed changes
Copy link
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

Wait for more approval.

wu-sheng
wu-sheng reviewed Jan 13, 2022
View reviewed changes
licenses/LICENSE-luaffi.txt Outdated
@@ -0,0 +1,58 @@
BSD License

For luaffifb software
Copy link
Member

@wu-sheng wu-sheng Jan 13, 2022

Choose a reason for hiding this comment

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

A thing, why do we have this update? I think we don't copy the luaffi source codes.

Copy link
Member Author

@alonelaval alonelaval Jan 13, 2022

Choose a reason for hiding this comment

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

ok

@wu-sheng wu-sheng merged commit 49adb5f into apache:master Jan 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wu-sheng wu-sheng wu-sheng approved these changes

@dmsolr dmsolr dmsolr approved these changes

@moonming moonming Awaiting requested review from moonming

+1 more reviewer

@membphis membphis membphis approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

1.0.0

Development

Successfully merging this pull request may close these issues.

5 participants

@alonelaval @wu-sheng @tzssangglass @membphis @dmsolr