-
Notifications
You must be signed in to change notification settings - Fork 78
Fix #571: Respect logLevel setting in pack tasks #591
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
base: main
Are you sure you want to change the base?
Conversation
The pack tasks now properly check the logLevel configuration before outputting info and debug messages. This allows users to suppress verbose logging by setting: - (Compile / pack / logLevel) := Level.Warn - (Compile / packCopyDependencies / logLevel) := Level.Warn This fixes the regression introduced in version 0.17 where Def.derive() prevented the logLevel setting from being properly scoped. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
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.
Summary of Changes
Hello @xerial, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request addresses a long-standing issue in the sbt-pack plugin where the logLevel setting was not being respected by pack tasks. By introducing explicit checks against the configured log level before emitting messages, this change allows users to effectively control the verbosity of logging output during packaging operations, improving the user experience and reducing unnecessary console noise.
Highlights
- Resolved Logging Inconsistency: Fixed an issue (reported as #571) where
packtasks insbt-packignored thelogLevelconfiguration, leading to verbose output even when suppressed. - Granular Log Control: Implemented conditional logging statements across various
packandpackCopyDependenciestasks, ensuring thatinfo,debug, andwarnmessages are only displayed if the configuredlogLevelpermits. - Comprehensive Test Coverage: Introduced a new scripted test suite specifically designed to validate that the
logLevelsetting is correctly applied and respected by thepacktasks.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
This pull request correctly fixes an issue where the logLevel setting was not being respected in pack tasks by adding explicit log level checks. The inclusion of a scripted test is a great addition to verify the fix. My review includes a suggestion to refactor the new logging logic to reduce code duplication and improve maintainability, as well as a minor correction for the test's build configuration to avoid potential confusion.
| packEnvVars := Map.empty, | ||
| Def.derive(pack := { | ||
| val out = streams.value | ||
| val level = (configuration / logLevel).value |
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.
To reduce the boilerplate of repeated if (level <= ...) checks, you could define helper methods for logging at the start of the pack task. This would make the code cleaner and more maintainable. Using by-name parameters (=> String) for the message will also prevent string construction if the log level is disabled.
You can then replace the if statements throughout this task with calls like infoLog(...).
This same pattern can be applied to the packModuleEntries and packCopyDependencies tasks to improve their readability as well.
| val level = (configuration / logLevel).value | |
| val level = (configuration / logLevel).value | |
| val log = out.log | |
| def infoLog(message: => String): Unit = if (level <= Level.Info) log.info(message) | |
| def warnLog(message: => String): Unit = if (level <= Level.Warn) log.warn(message) |
|
|
||
| // Test that logLevel setting is respected | ||
| // When set to Warn, info messages should not appear | ||
| Test / logLevel := Level.Warn No newline at end of 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.
The logLevel setting here is scoped to the Test configuration, which means it will affect tasks like test, but not the pack tasks which are typically in the Compile scope. The scripted test file correctly sets Compile / pack / logLevel, so this line seems to be either redundant or not having the intended effect described in the comment. If the goal is to test the pack tasks, this line can probably be removed to avoid confusion, as the test script handles the log level setting explicitly.
- Add LevelAwareLogger helper class to centralize log level checking - Simplify all logging statements by removing redundant level checks - Maintain compatibility while making code more maintainable
Summary
Problem
Since version 0.17, the
logLevelsetting wasn't being respected in pack tasks due to the use ofDef.derive(). Users couldn't suppress verbose logging by setting(pack / logLevel) := Level.Warn.Solution
Modified all logging statements in pack-related tasks to check the configured log level before outputting messages. This allows users to control log verbosity by setting:
Test plan
src/sbt-test/sbt-pack/log-level🤖 Generated with Claude Code