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

GH-661 Escalate Ignore developer API #703

Merged
merged 29 commits into from
Jun 29, 2024
Merged

GH-661 Escalate Ignore developer API #703

merged 29 commits into from
Jun 29, 2024

Conversation

Jakubk15
Copy link
Member

No description provided.

@Jakubk15 Jakubk15 linked an issue Feb 24, 2024 that may be closed by this pull request
Ignore System API (Manage of an ignore system via service) #661
Closed
@Jakubk15 Jakubk15 changed the title Escalate Ignore developer API GH-661 Escalate Ignore developer API Feb 24, 2024
P1otrulla
P1otrulla approved these changes Feb 24, 2024
View reviewed changes
Copy link
Member

@P1otrulla P1otrulla left a comment

Choose a reason for hiding this comment

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

good job, Create please example in api for IgnoreService#purge in command

CitralFlo
CitralFlo approved these changes Feb 25, 2024
View reviewed changes
Copy link
Member

@CitralFlo CitralFlo left a comment

Choose a reason for hiding this comment

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

LGTM, one question thou

eternalcore-api/src/main/java/com/eternalcode/core/feature/ignore/event/IgnoreEvent.java Outdated Show resolved Hide resolved
vLuckyyy
vLuckyyy requested changes Feb 26, 2024
View reviewed changes
Copy link
Member

@vLuckyyy vLuckyyy left a comment

Choose a reason for hiding this comment

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

Add examples in eternalcore-api-example module.

vLuckyyy
vLuckyyy requested changes Feb 29, 2024
View reviewed changes
Copy link
Member

@vLuckyyy vLuckyyy left a comment

Choose a reason for hiding this comment

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

Add examples for ignore events

Rollczi
Rollczi requested changes Feb 29, 2024
View reviewed changes
Copy link
Member

@Rollczi Rollczi left a comment

Choose a reason for hiding this comment

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

eternalcore-api/src/main/java/com/eternalcode/core/feature/ignore/event/IgnoreEvent.java Outdated Show resolved Hide resolved
@vLuckyyy vLuckyyy requested a review from Rollczi June 11, 2024 10:12
@Jakubk15 Jakubk15 requested a review from vLuckyyy June 23, 2024 19:22
@Jakubk15
Copy link
Member Author

One more issue though; when we pass the Boolean through CompletableFuture to the implementing class, such as IgnoreCommand, and when for example the event was cancelled, no message is shown to the player. But we also use the cancelled parameter as indicator if for example the player was null. I suggest we convert CompletableFuture<Boolean> to CompletableFuture<Result<Boolean, EventFailureReason>> to extract this information and create appropriate message to the player.

@Jakubk15
Copy link
Member Author

@EternalCodeTeam/student @EternalCodeTeam/team thoughts on this ^

@vLuckyyy vLuckyyy merged commit 484069b into master Jun 29, 2024
3 checks passed
@Rollczi Rollczi deleted the add-ignore-api branch June 29, 2024 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vLuckyyy vLuckyyy vLuckyyy approved these changes

@igoyek igoyek igoyek approved these changes

@P1otrulla P1otrulla P1otrulla approved these changes

@CitralFlo CitralFlo CitralFlo approved these changes

@VerniQ VerniQ VerniQ approved these changes

@Kazaruma Kazaruma Kazaruma approved these changes

@Kamicjusz Kamicjusz Awaiting requested review from Kamicjusz

@Rollczi Rollczi Awaiting requested review from Rollczi

Assignees
No one assigned
Labels
🔓 API
Projects
EternalCore
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Ignore System API (Manage of an ignore system via service)
8 participants
@Jakubk15 @Rollczi @vLuckyyy @igoyek @P1otrulla @CitralFlo @VerniQ @Kazaruma