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

Add PlaceholderAPI support #132

Merged
merged 4 commits into from
Nov 15, 2019
Merged

Add PlaceholderAPI support #132

merged 4 commits into from
Nov 15, 2019

Conversation

svnxd
Copy link
Contributor

@svnxd svnxd commented May 30, 2019

No description provided.

Perksey
Perksey requested changes May 30, 2019
View reviewed changes
Copy link
Contributor

@Perksey Perksey left a comment

Choose a reason for hiding this comment

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

A few things. Also have you tested this?

...-spigot/src/main/java/tech/mcprison/prison/spigot/placeholder/PlaceHolderAPIIntegration.java Show resolved Hide resolved
...-spigot/src/main/java/tech/mcprison/prison/spigot/placeholder/PlaceHolderAPIIntegration.java Outdated Show resolved Hide resolved
@Perksey Perksey requested a review from fynntimes May 30, 2019 15:46
fynntimes
fynntimes requested changes May 30, 2019
View reviewed changes
Copy link
Member

@fynntimes fynntimes left a comment

Choose a reason for hiding this comment

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

Very nice! I included some changes; also, is this tested?

prison-spigot/build.gradle Outdated Show resolved Hide resolved
...-spigot/src/main/java/tech/mcprison/prison/spigot/placeholder/PlaceHolderAPIIntegration.java Show resolved Hide resolved
@Perksey
Copy link
Contributor

Perksey commented Jul 7, 2019

@faizaand follow-up review?

@Perksey Perksey requested a review from fynntimes August 28, 2019 15:04
@Perksey
Copy link
Contributor

Perksey commented Nov 15, 2019

Merging as all review comments have been rectified.

@Perksey Perksey merged commit 91f88db into PrisonTeam:master Nov 15, 2019
rbluer added a commit to rbluer/Prison that referenced this pull request Nov 28, 2019
@rbluer
Fixes the PrisonTeam#132 pull request.
760f6e2 
It causes prison to fail on a clean server and throws a class not found exception.
The new library needs compile instead of compileOnly and it must also be shadowed.
@rbluer
Copy link
Collaborator

rbluer commented Nov 28, 2019 •
edited
Loading

This code was not tested on a clean severer, and as such it caused the prison plugin to fail.
It passed a simple compile with Gradle. It passed appveyor. But caused a class not found exception to be thrown on plugin's onEnable. I tested on 1.8.8 and 1.14.4 and confirmed the failure on both platforms. Failures would have happened on other versions too.
I see both faizaand and perkesy asked if it was tested, bvnxd never confirmed that they tested it.
Would have been bad if someone tried to load the build artifact on their server.
I fixed gradle configs so the required classes are shadowed now.

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

@Perksey Perksey Perksey approved these changes

@fynntimes fynntimes Awaiting requested review from fynntimes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@svnxd @Perksey @rbluer @fynntimes