cmd progress for git solution download#4051
Conversation
WalkthroughThe pull request introduces enhancements to logging and progress reporting across multiple files in the Ginger project. In the Changes
Possibly related PRs
Suggested reviewers
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
Ginger/GingerCoreCommon/ReporterLib/Reporter.cs (1)
40-40: Add XML documentation for the new parameter.The method signature has been updated, but the XML documentation should be updated to describe the new
overwriteCurrentLineparameter.Add the following XML documentation:
+/// <param name="overwriteCurrentLine">When true, overwrites the current console line instead of appending a new line.</param> public static void ToLog(eLogLevel logLevel, string messageToLog, Exception exceptionToLog = null, TelemetryMetadata metadata = null, Boolean overwriteCurrentLine = false)Also applies to: 54-54, 56-59
Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs (1)
573-573: Remove commented code.The commented line
//Console.WriteLine();should be removed as it serves no purpose.🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis
[warning] 573-573: Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs#L573
Remove this commented out code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Ginger/GingerCoreCommon/ReporterLib/Reporter.cs(4 hunks)Ginger/GingerCoreCommon/ReporterLib/WorkSpaceReporterBase.cs(3 hunks)Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs(3 hunks)
🧰 Additional context used
🪛 GitHub Check: Codacy Static Code Analysis
Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs
[warning] 573-573: Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs#L573
Remove this commented out code.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
Ginger/GingerCoreCommon/ReporterLib/WorkSpaceReporterBase.cs (2)
35-35: LGTM!The field declaration follows naming conventions and has appropriate access level.
37-37: LGTM!The method signature change is backward compatible and the parameter name is descriptive.
Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs (2)
569-569: LGTM!The event handler is properly subscribed and unsubscribed, following best practices.
Also applies to: 579-579
591-594: LGTM!The XML documentation is well-written and follows documentation standards.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs (1)
568-568: Move success message inside try block.The success message is logged after unsubscribing from the event, but it's outside the try block. This means it will be logged even if an exception occurs during the download process.
Apply this diff to fix the issue:
progressNotifier.ProgressUpdated -= ProgressNotifier_ProgressUpdated; - Reporter.ToLog(eLogLevel.INFO, "Solution downloaded/updated successfully"); + if (solutionDownloadedSuccessfully) + { + Reporter.ToLog(eLogLevel.INFO, "Solution downloaded/updated successfully"); + }Also applies to: 578-579
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Build Stage / build
🔇 Additional comments (1)
Ginger/GingerCoreNET/RunLib/CLILib/CLIHelper.cs (1)
589-602: Fix integer division and add null check.The progress calculation has potential issues with integer division truncation and division by zero.
This issue was previously identified. Please refer to the existing comment for the recommended fix.
2 participants
Thank you for your contribution.
Before submitting this PR, please make sure:
Summary by CodeRabbit
New Features
Bug Fixes
Refactor