feat: dont print log when running commands unless error #800
Conversation
WalkthroughThe pull request introduces changes to the Goravel framework's console and environment handling. The primary modification is the removal of the boolean parameter from the Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant Env as Environment
participant Args as Command Line Args
App->>Env: Check IsArtisan()
Env->>Args: Iterate through os.Args
Args-->>Env: Return Artisan status
Env-->>App: Set application mode
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #800 +/- ##
==========================================
- Coverage 69.78% 69.73% -0.05%
==========================================
Files 218 218
Lines 18858 18866 +8
==========================================
- Hits 13160 13157 -3
- Misses 4984 4991 +7
- Partials 714 718 +4 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
support/env/env.go (1)
28-28: Add documentation comment for IsArtisanFor consistency with other functions in this file, please add a documentation comment explaining the purpose of IsArtisan.
+// IsArtisan checks if the application is running in Artisan mode. func IsArtisan() bool {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
console/application.go(2 hunks)console/application_test.go(1 hunks)console/cli_helper_test.go(1 hunks)console/service_provider.go(1 hunks)database/gorm/logger.go(1 hunks)support/env/env.go(1 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
support/env/env.go
[warning] 31-32: support/env/env.go#L31-L32
Added lines #L31 - L32 were not covered by tests
console/service_provider.go
[warning] 20-20: console/service_provider.go#L20
Added line #L20 was not covered by tests
database/gorm/logger.go
[warning] 26-27: database/gorm/logger.go#L26-L27
Added lines #L26 - L27 were not covered by tests
🔇 Additional comments (9)
console/cli_helper_test.go (1)
16-16: Good update to match the new function signature.
Removing the boolean parameter from NewApplication aligns with the recent refactor and simplifies the interface.
database/gorm/logger.go (2)
17-17: Importing the env package here is appropriate.
This import enables the logger to rely on the environment check for Artisan mode, helping unify the logic around logging levels.
25-27: Lines lack test coverage.
Forcing the logger to logger.Error under Artisan mode achieves the PR’s objective of minimizing console log noise. However, static analysis indicates that these lines are not covered by tests, which may reduce overall coverage and confidence in this behavior.
Would you like assistance adding a test to ensure these lines are covered?
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 26-27: database/gorm/logger.go#L26-L27
Added lines #L26 - L27 were not covered by tests
console/application.go (3)
11-11: New import of env package is consistent with the design.
Centralizing the Artisan check in the environment logic helps keep the application setup clean.
21-21: Removal of the boolean parameter streamlines the constructor.
This makes the function signature more intuitive, avoiding knowledge duplication about Artisan mode.
32-32: Setting isArtisan via env.IsArtisan() is more maintainable.
Delegating the Artisan detection to the environment layer centralizes configuration and reduces potential inconsistencies.
console/application_test.go (1)
16-16: Consistent change to the NewApplication call.
This mirrors the updated signature that uses env.IsArtisan() instead of a boolean parameter.
console/service_provider.go (2)
20-20: Verify alignment with PR objective
The removal of the boolean parameter seems to be part of a larger change to control log output. Please verify that this change, combined with env.IsArtisan(), properly supports the PR's objective of suppressing logs during command execution.
✅ Verification successful
The removal of the boolean parameter is properly aligned with the PR's objective
The verification shows that the change is well-integrated with the logging control mechanism:
- The
isArtisanfield in the Application struct is now automatically set viaenv.IsArtisan()during initialization - This field is used consistently throughout the application to control command execution behavior
- The removal of the explicit boolean parameter simplifies the API while maintaining the same functionality through environment-based control
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for logging-related code that might be affected
rg -A 3 "log\." "console/"
Length of output: 26
Script:
#!/bin/bash
# Search for logging-related code with broader patterns
rg -A 3 -i "log" "console/"
# Look for env.IsArtisan usage
rg -A 3 "IsArtisan" "console/"
# Check the removed parameter's history
git log -L :NewApplication:console/service_provider.go
# Look at the full content of service_provider.go
cat console/service_provider.go
Length of output: 1599
Script:
#!/bin/bash
# Check the application.go file for logging control logic
cat console/application.go
# Look for any other files that might contain logging control
rg -A 3 "isArtisan" .
Length of output: 6094
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 20-20: console/service_provider.go#L20
Added line #L20 was not covered by tests
20-20: Add test coverage for the updated NewApplication call
The modified NewApplication call isn't covered by tests. Please add test cases to verify the behavior with the new signature.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 20-20: console/service_provider.go#L20
Added line #L20 was not covered by tests
📑 Description
Don't print the log when running commands unless there is an error.
Ran installer success:
Summary by CodeRabbit
New Features
IsArtisan()function to detect Artisan environment based on command-line argumentsRefactor
NewApplicationfunction by removing debug/Artisan boolean parameterenv.IsArtisan()methodChores
✅ Checks