Skip to content

RemovedUnusedMethod#4377

Merged
ravirk91 merged 7 commits into
masterfrom
GitHubAlertFix-74
Dec 9, 2025
Merged

RemovedUnusedMethod#4377
ravirk91 merged 7 commits into
masterfrom
GitHubAlertFix-74

Conversation

@rathimayur

@rathimayur rathimayur commented Dec 9, 2025 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Description

Type of Change

  • Bug fix - [ ] New feature - [ ] Breaking change - [ ] Plugin update

Checklist

  • PR description clearly describes the changes
  • Target branch is correct (master for features, Releases/* for fixes)
  • Latest code from target branch merged
  • No commented/junk code included
  • No new build warnings or errors
  • All existing unit tests pass
  • New unit tests added for new functionality
  • Cross-platform compatibility verified (Windows/Linux/macOS)
  • CI/CD pipeline passes
  • Code follows project conventions (Act{Platform}{Type}, {Platform}Driver)
  • Repository objects use [IsSerializedForLocalRepository] where needed
  • Error handling uses Reporter.ToLog() pattern
  • Documentation updated for user-facing changes
  • Self-review completed and code review comments addressed

Summary by CodeRabbit

  • Refactor

    • Streamlined internal variable export flow, removed legacy export routines, and eliminated creation of temporary export files to simplify maintenance.

  • Bug Fixes / Security

    • Variable export now omits password-type variables, preventing sensitive values from being included in exported outputs.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Dec 9, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Walkthrough

Removed two public variable-export helper methods from ActRobot, added LINQ-based filtering to exclude password variables, and updated Execute to stop creating temporary CSV/JSON files and to use the filtered variable list when assembling Robot execution parameters.

Changes

Cohort / File(s) Summary
ActRobot core change
Ginger/GingerCoreNET/ActionsLib/RobotFramework/ActRobot.cs		 Removed public methods WriteVariablesToCSVFile and WriteVariablesToJSONFile. Added using GingerCore.Variables and using System.Linq. GetCreateBusinessAndActivityVariablesToJSONList now filters out VariablePasswordString. Execute no longer creates temporary CSV/JSON files or calls the removed writers; it assembles Robot command parameters using the filtered variable list. Removed Newtonsoft.Json usage/import.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify there are no external usages of the removed public methods across the repository.
  • Confirm the LINQ filter correctly excludes only password variables and preserves expected variable ordering/semantics.
  • Check Execute’s parameter assembly for any platform/path or quoting edge cases now that temp file creation is removed.

Suggested reviewers

  • Maheshkale447

Poem

🐰 I nibble through code at break of day,
Two old helpers hopped away,
Passwords tucked where none can peep,
Robot strides on, tidy and spry — no heap. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is completely empty—it contains only the template structure with no actual content describing the changes, type of change, or any checklist items checked. Fill in the Description section with details about the removed methods, select the appropriate Type of Change option, and complete the relevant checklist items to document the changes and review status.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'RemovedUnusedMethod' is generic and vague. It does not clearly specify which methods were removed or why, making it difficult to understand the main change from scanning history. Provide a more specific title that identifies what was removed, such as 'Remove unused WriteVariables CSV/JSON methods from ActRobot' or 'Remove unused file-writing methods from ActRobot class'.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch GitHubAlertFix-74
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8909917 and f79bd52.

📒 Files selected for processing (1)
  • Ginger/GingerCoreNET/ActionsLib/RobotFramework/ActRobot.cs (4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use Reporter.ToLog(eLogLevel.ERROR, message) for logging errors

Files:

  • Ginger/GingerCoreNET/ActionsLib/RobotFramework/ActRobot.cs
🧠 Learnings (9)
📚 Learning: 2025-11-28T05:32:43.529Z 
Learnt from: CR
Repo: Ginger-Automation/Ginger PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T05:32:43.529Z
Learning: Applies to **/{GingerCore,GingerCoreNET,GingerCoreCommon}/Actions/Act*.cs : Name actions following the pattern: `Act{PlatformType}{ActionType}` (e.g., `ActBrowserElement`, `ActConsoleCommand`)

Applied to files:

  • Ginger/GingerCoreNET/ActionsLib/RobotFramework/ActRobot.cs
📚 Learning: 2025-11-28T05:32:43.529Z 
Learnt from: CR
Repo: Ginger-Automation/Ginger PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T05:32:43.529Z
Learning: Applies to **/{GingerCore,GingerCoreNET,GingerCoreCommon}/Actions/Act*.cs : Actions should inherit from `Act` class and implement platform-specific logic

Applied to files:

  • Ginger/GingerCoreNET/ActionsLib/RobotFramework/ActRobot.cs
📚 Learning: 2025-04-25T13:29:45.059Z 
Learnt from: GokulBothe99
Repo: Ginger-Automation/Ginger PR: 4188
File: Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs:22-23
Timestamp: 2025-04-25T13:29:45.059Z
Learning: The `using Amdocs.Ginger.Repository;` statement in Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs is necessary as it provides access to the `ObservableList<>` class which is used throughout the file for collections of RunSetConfig, AnalyzerItemBase, and ApplicationPOMModel objects.

Applied to files:

  • Ginger/GingerCoreNET/ActionsLib/RobotFramework/ActRobot.cs
📚 Learning: 2025-04-25T13:29:45.059Z 
Learnt from: GokulBothe99
Repo: Ginger-Automation/Ginger PR: 4188
File: Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs:22-23
Timestamp: 2025-04-25T13:29:45.059Z
Learning: The `using Amdocs.Ginger.Repository;` statement in Ginger/GingerCoreNET/RunLib/CLILib/DoOptionsHanlder.cs is necessary as it imports the ObservableList<> class which is used throughout the file.

Applied to files:

  • Ginger/GingerCoreNET/ActionsLib/RobotFramework/ActRobot.cs
📚 Learning: 2025-03-20T11:10:30.816Z 
Learnt from: GokulBothe99
Repo: Ginger-Automation/Ginger PR: 4137
File: Ginger/Ginger/SourceControl/SourceControlProjectsPage.xaml.cs:21-21
Timestamp: 2025-03-20T11:10:30.816Z
Learning: The `Amdocs.Ginger.Common.SourceControlLib` namespace is required in files that reference the `GingerSolution` class for source control operations in the Ginger automation framework.

Applied to files:

  • Ginger/GingerCoreNET/ActionsLib/RobotFramework/ActRobot.cs
📚 Learning: 2025-06-16T10:37:13.073Z 
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4232
File: Ginger/GingerCoreNET/ActionsLib/UI/VisualTesting/VRTAnalyzer.cs:19-22
Timestamp: 2025-06-16T10:37:13.073Z
Learning: In Ginger codebase, both `using amdocs.ginger.GingerCoreNET;` and `using Amdocs.Ginger.CoreNET;` are valid and serve different purposes. The first (lowercase) contains the WorkSpace class and related workspace functionality, while the second (proper case) contains drivers like GenericAppiumDriver and other core functionality. Both may be required in files that use types from both namespaces.

Applied to files:

  • Ginger/GingerCoreNET/ActionsLib/RobotFramework/ActRobot.cs
📚 Learning: 2025-06-16T09:52:40.873Z 
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 4232
File: Ginger/GingerCoreNET/ActionsLib/UI/VisualTesting/VRTAnalyzer.cs:19-22
Timestamp: 2025-06-16T09:52:40.873Z
Learning: The `GenericAppiumDriver` class is located in the `Amdocs.Ginger.CoreNET` namespace, not in a namespace that matches its file path structure. File paths don't always correspond to namespace declarations in this codebase.

Applied to files:

  • Ginger/GingerCoreNET/ActionsLib/RobotFramework/ActRobot.cs
📚 Learning: 2024-11-06T09:06:34.050Z 
Learnt from: prashelke
Repo: Ginger-Automation/Ginger PR: 3984
File: Ginger/GingerCoreNET/ALMLib/RQM/ExportToRQM.cs:911-911
Timestamp: 2024-11-06T09:06:34.050Z
Learning: In the `Ginger/GingerCoreNET/ALMLib/RQM/ExportToRQM.cs` file of the Ginger project, initializing lists in C# using `[]` syntax (e.g., `List<T> list = [];`) is acceptable and does not require changing to `new List<T>()`.

Applied to files:

  • Ginger/GingerCoreNET/ActionsLib/RobotFramework/ActRobot.cs
📚 Learning: 2025-08-25T08:48:11.915Z 
Learnt from: AmanPrasad43
Repo: Ginger-Automation/Ginger PR: 4281
File: Ginger/GingerCore/GingerCore.csproj:538-538
Timestamp: 2025-08-25T08:48:11.915Z
Learning: In the Ginger codebase, the OWASPZAPDotNetAPI package reference in GingerCore.csproj is required for architectural reasons, even when direct usage is not immediately visible in the codebase. This was confirmed by AmanPrasad43 during the security testing feature implementation.

Applied to files:

  • Ginger/GingerCoreNET/ActionsLib/RobotFramework/ActRobot.cs
🔇 Additional comments (3)
Ginger/GingerCoreNET/ActionsLib/RobotFramework/ActRobot.cs (3)

23-23: LGTM! Using directives support the new filtering logic.

The added using directives are necessary for filtering password variables using LINQ and type checking with VariablePasswordString.

Also applies to: 28-28

127-164: Excellent security improvement!

Filtering out password variables across solution, business flow, and activity scopes prevents sensitive data from being exposed in the JSON variable file. The use of pattern matching (vrb is not VariablePasswordString) is clean and appropriate.

1-437: No action required—removed methods are not referenced in the codebase.

The search confirms that the removed public methods WriteVariablesToCSVFile and WriteVariablesToJSONFile have no usages elsewhere in the codebase. Their removal is safe and does not introduce breaking changes. The replacement method WriteVariablesToJSONFile_V2 is already implemented and functional.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Dec 9, 2025
View reviewed changes
coderabbitai[bot]
coderabbitai Bot previously approved these changes Dec 9, 2025
View reviewed changes
coderabbitai[bot]
coderabbitai Bot previously approved these changes Dec 9, 2025
View reviewed changes
@ravirk91 ravirk91 self-assigned this Dec 9, 2025
@ravirk91
ravirk91 merged commit b1b6c93 into master Dec 9, 2025
14 checks passed
@ravirk91
ravirk91 deleted the GitHubAlertFix-74 branch December 9, 2025 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] approved these changes

Assignees

@ravirk91 ravirk91

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@rathimayur @ravirk91