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.

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

Plugins: Remove support for onModule #21416

Merged
merged 3 commits into from Nov 23, 2016
Merged

Plugins: Remove support for onModule #21416

merged 3 commits into from Nov 23, 2016

Conversation

rjernst
Copy link
Member

@rjernst rjernst commented Nov 8, 2016

All plugin extension points have been converted to pull based
interfaces. This change removes the infrastructure for the black-magic
onModule methods.

@rjernst
Plugins: Remove support for onModule
3fda76b 
All plugin extension points have been converted to pull based
interfaces. This change removes the infrastructure for the black-magic
onModule methods.
@rjernst rjernst added :Core/Infra/Plugins Plugin API and infrastructure v6.0.0-alpha1 v5.1.1 labels Nov 8, 2016
@clintongormley
Copy link

w00t. Do we need breaking java changes in the docs?

@nik9000
Copy link
Member

nik9000 commented Nov 9, 2016

w00t. Do we need breaking java changes in the docs?

Yes.

@rjernst
Copy link
Member Author

rjernst commented Nov 15, 2016

Regarding the breaking changes, since this will go back to 5.1, should it just go into the backport since there are no 5.1 breaking changes in master?

@jasontedor
Copy link
Member

Regarding the breaking changes, since this will go back to 5.1, should it just go into the backport since there are no 5.1 breaking changes in master?

That's exactly how I handle these sorts of things.

@s1monw
Copy link
Contributor

s1monw commented Nov 15, 2016

hell yeah!

@rjernst
Copy link
Member Author

rjernst commented Nov 15, 2016

retest this please

@rjernst
Copy link
Member Author

rjernst commented Nov 15, 2016

@nik9000 @jasontedor @s1monw Do you have any technical comments on this PR or is it just needing the breaking changes docs when backported?

@jasontedor
Copy link
Member

Do you have any technical comments on this PR or is it just needing the breaking changes docs when backported?

Just that, the PR looks good to me. I'll leave a formal LGTM.

jasontedor
jasontedor approved these changes Nov 15, 2016
View reviewed changes
Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

LGTM.

nik9000
nik9000 approved these changes Nov 15, 2016
View reviewed changes
Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

Same here. Change looks fine.

@rjernst rjernst merged commit 10a945a into elastic:master Nov 23, 2016
rjernst added a commit that referenced this pull request Nov 23, 2016
@rjernst
Plugins: Remove support for onModule (#21416)
8f46d04 
All plugin extension points have been converted to pull based
interfaces. This change removes the infrastructure for the black-magic
onModule methods.
@rjernst rjernst deleted the on_module_goes_boom branch November 23, 2016 07:16
@rjernst
Copy link
Member Author

rjernst commented Nov 23, 2016

With the backport to 5.x, I pushed 2162a6d with a breaking changes note. /cc @clintongormley

@clintongormley
Copy link

thanks @rjernst

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nik9000 nik9000 nik9000 approved these changes

@jasontedor jasontedor jasontedor approved these changes

Assignees
No one assigned
Labels
>breaking-java :Core/Infra/Plugins Plugin API and infrastructure v5.1.1 v6.0.0-alpha1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@rjernst @clintongormley @nik9000 @jasontedor @s1monw