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

FullHistory #623

Merged
merged 9 commits into from
Jul 5, 2023
Merged

FullHistory #623

merged 9 commits into from
Jul 5, 2023

Conversation

tWizT3ddreaMr
Copy link
Contributor

took a more configurable way for #427 where you can exclude any punishment type if you so choose

took a more configurable way for DevLeoko#427 where you can exclude any punishment type if you so choose
These are changes based on the comments of the pull request
Added a lime to conform to  style, and adjusted the javadoc
@tWizT3ddreaMr
Copy link
Contributor Author

tWizT3ddreaMr commented Jul 4, 2023

That makes sense, I made the changes based on what I interpreted of what you said.
I tried to fix the new javadoc for it, but I feel like i may have left it lacking
I had to rearrange the order for the new method to avoid ambiguousness since there is going to actively be nulls put in

I condensed the new method i added wouldnt let me add the ternary operator in the getpunishments
And i got rid of the temp variable because I didn't like it
@DevLeoko
Copy link
Owner

DevLeoko commented Jul 4, 2023

Feel free to also message me on Discord if you have questions or want to chat about the comments.
I appreciate your contribution 👍

These are the asked changes, however it cannot stay this way because ambiguous callls
Removing ambiguous calls
Comment on lines 301 to 304
if (!Universal.get().hasPerms(input.getSender(), "ab.history")) {
MessageManager.sendMessage(input.getSender(), "General.NoPerms", true);
return;
}
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think that you need to perform a permission check here as the permission for the command is specified a few lines above

@DevLeoko DevLeoko merged commit df18f2f into DevLeoko:development Jul 5, 2023
@DevLeoko
Copy link
Owner

DevLeoko commented Jul 5, 2023

Thanks for adding this feature

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants