-
-
Notifications
You must be signed in to change notification settings - Fork 104
Feature/logging-for-disabled-features #1048
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
Feature/logging-for-disabled-features #1048
Conversation
christolis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welcome to the TJ-Bot project! I left some review comments. It looks like the pre-commit hook didn't execute when making the commit, so try running the spotlessApply task from Gradle (./gradlew spotlessApply) to fix this and so that the tests pass. 🙌
application/src/main/java/org/togetherjava/tjbot/config/FeatureBlacklist.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/config/FeatureBlacklist.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/features/code/CodeMessageHandler.java
Outdated
Show resolved
Hide resolved
|
@christolis I have done the changes. |
christolis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Well done 🚀
This is continuing #898 - which is the logging for disabled features (I am Tea from the discord server)
I was implementing the change requests at #899 from the assignees code since that PR has been quite inactive, so i've added only small modifcations.
Although, one change requested to use "Stream<? extends F>" at the disableMatching method at FeatureBlacklist class, however this was causing some problems since at the Features class the return type of its method is just "Features" so i am unsure how to go forward with that particular modifcation.
However everything works and I tested it
Credit to @adiChoudhary for doing most of it at #899 , i just finished things off