-
-
Notifications
You must be signed in to change notification settings - Fork 0
GH-120 Add debug mode #120
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
Conversation
WalkthroughThis pull request introduces a debug mode into the plugin by adding a new boolean configuration flag and integrating conditional logging. A public static logger, Assessment against linked issues
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/com/eternalcode/parcellockers/database/wrapper/AbstractRepositoryOrmLite.java (1)
73-75
: Consider adding logging to selectAll method for consistencyAll other methods have logging added, but selectAll still uses the direct reference. Consider adding logging here too.
protected <T> CompletableFuture<List<T>> selectAll(Class<T> type) { - return this.action(type, Dao::queryForAll); + return this.action(type, dao -> { + ParcelLockers.DEBUG_LOGGER.info("Selecting all entities of type: {}", type.getSimpleName()); + return dao.queryForAll(); + }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/eternalcode/parcellockers/ParcelLockers.java
(3 hunks)src/main/java/com/eternalcode/parcellockers/configuration/implementation/PluginConfiguration.java
(1 hunks)src/main/java/com/eternalcode/parcellockers/database/wrapper/AbstractRepositoryOrmLite.java
(2 hunks)src/main/java/com/eternalcode/parcellockers/util/NullLogger.java
(1 hunks)
🔇 Additional comments (13)
src/main/java/com/eternalcode/parcellockers/configuration/implementation/PluginConfiguration.java (1)
42-44
: Good addition of debug mode settingThe new debug flag is well-documented with a clear description. Nice job setting the default to false to avoid flooding the console unnecessarily.
src/main/java/com/eternalcode/parcellockers/util/NullLogger.java (1)
1-312
: Good implementation of null logger patternThe NullLogger class properly implements all Logger interface methods with no-op implementations. This is a clean approach to handle disabled debug logging without conditional checks throughout the code.
src/main/java/com/eternalcode/parcellockers/database/wrapper/AbstractRepositoryOrmLite.java (8)
4-4
: Appropriate import addedThe import for ParcelLockers is needed to access the DEBUG_LOGGER.
25-28
: Good logging implementation for save methodThe logging statement provides useful context before performing the database operation.
32-35
: Good logging implementation for saveIfNotExist methodClear logging message with context about the operation and entity.
39-42
: Good logging implementation for select methodThe logging provides useful information about the ID being selected.
46-49
: Good logging implementation for selectSafe methodAppropriate logging with indication that it's in safe mode.
53-56
: Good logging implementation for delete methodClear logging of the entity being deleted.
60-63
: Good logging implementation for deleteAll methodSimple but informative logging message for bulk deletion.
67-70
: Good logging implementation for deleteById methodAppropriate logging with the ID being deleted.
src/main/java/com/eternalcode/parcellockers/ParcelLockers.java (3)
40-40
: Appropriate import addedGood addition of the NullLogger import to support the debug mode feature.
73-73
: Good addition of static logger fieldThe DEBUG_LOGGER is properly declared as public static to be accessible throughout the application.
113-117
: Well-implemented conditional logger initializationThe logger is properly initialized based on the debug setting. Using a NullLogger when debug is disabled is an efficient approach.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/com/eternalcode/parcellockers/database/wrapper/AbstractRepositoryOrmLite.java (1)
74-74
: Added logging in selectAll methodNice addition, but note this logging is placed outside the action lambda unlike other methods. This works fine but is slightly inconsistent with the pattern used elsewhere.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/eternalcode/parcellockers/database/wrapper/AbstractRepositoryOrmLite.java
(2 hunks)
🔇 Additional comments (8)
src/main/java/com/eternalcode/parcellockers/database/wrapper/AbstractRepositoryOrmLite.java (8)
4-4
: Import added for debug loggingThe import for ParcelLockers class is added to access the DEBUG_LOGGER.
25-28
: Added logging in save methodSimple logging added when saving entities to help with debugging database operations.
32-35
: Added logging in saveIfNotExist methodHelpful log statement with clear context about the save operation mode.
39-42
: Added logging in select methodBasic logging to track entity selection operations by ID.
46-49
: Added logging in selectSafe methodGood logging that clarifies this is using the safe mode (returning Optional).
53-56
: Added logging in delete methodSimple tracking of entity deletion operations.
60-63
: Added logging in deleteAll methodGood use of type.getSimpleName() to show which entity type is being deleted.
67-70
: Added logging in deleteById methodSimple tracking for deletion by ID operations.
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/com/eternalcode/parcellockers/ParcelLockers.java (2)
73-73
: Added static debug loggerThe logger is properly initialized with a no-operation logger. Consider adding a brief comment to explain its purpose for easier understanding by other developers.
- public static org.slf4j.Logger DEBUG_LOGGER = NOPLogger.NOP_LOGGER; + // Logger for debug mode, only logs when debug is enabled in config + public static org.slf4j.Logger DEBUG_LOGGER = NOPLogger.NOP_LOGGER;
113-115
: Added debug mode configurationGood implementation of the debug mode toggle. The logger gets properly initialized when debug is enabled in the configuration.
if (config.settings.debug) { - DEBUG_LOGGER = org.slf4j.LoggerFactory.getLogger("ParcelLockers] [DEBUG"); + DEBUG_LOGGER = org.slf4j.LoggerFactory.getLogger("ParcelLockers-DEBUG"); }The logger name format is a bit unusual. Consider using a more standard naming format.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/eternalcode/parcellockers/ParcelLockers.java
(2 hunks)
🔇 Additional comments (1)
src/main/java/com/eternalcode/parcellockers/ParcelLockers.java (1)
69-69
: Added import for NOPLoggerGood addition of the import needed for the new debug logger functionality.
Closes #95