-
Notifications
You must be signed in to change notification settings - Fork 0
refactor #13
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
refactor #13
Conversation
WalkthroughUpgrades CI workflows to GitHub Actions v6 and .NET 8, restructures template metadata and derived symbols, reconfigures test projects and analyzers, moves/removes setup scripts and license, consolidates StyleCop suppressions, adjusts packaging, and replaces/moves ErrorHandler along with README and .gitignore updates. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (1)Frends.*/**/*.cs⚙️ CodeRabbit configuration file
Files:
🪛 markdownlint-cli2 (0.18.1)README.md45-45: Fenced code blocks should have a language specified (MD040, fenced-code-language) 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. Comment |
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: 4
🧹 Nitpick comments (1)
Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute/Frends.Echo.Execute.cs (1)
21-21: Consider a more descriptive placeholder.The placeholder "x." is unclear. Consider using something more descriptive like "Additional options." or "Options description." to better guide template users.
Apply this diff to improve clarity:
- /// <param name="options">x.</param> + /// <param name="options">Additional options.</param>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
.github/workflows/AutoBuild.yml(2 hunks).github/workflows/BuildMaster.yml(2 hunks).gitignore(1 hunks)Frends.Template/.github/workflows/Execute_release.yml(1 hunks)Frends.Template/.github/workflows/Execute_test_on_main.yml(1 hunks)Frends.Template/.github/workflows/Execute_test_on_push.yml(1 hunks)Frends.Template/.template.config/template.json(2 hunks)Frends.Template/Apache-2.0(0 hunks)Frends.Template/Frends.Echo.Execute.Tests/Frends.Echo.Execute.Tests.csproj(0 hunks)Frends.Template/Frends.Echo.Execute.Tests/GlobalSuppressions.cs(0 hunks)Frends.Template/Frends.Echo.Execute/.gitignore(1 hunks)Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute.Tests/Frends.Echo.Execute.Tests.csproj(1 hunks)Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute.Tests/GlobalSuppressions.cs(1 hunks)Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute.sln(1 hunks)Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute/Definitions/Options.cs(1 hunks)Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute/Definitions/Result.cs(1 hunks)Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute/Frends.Echo.Execute.cs(1 hunks)Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute/Frends.Echo.Execute.csproj(1 hunks)Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute/GlobalSuppressions.cs(1 hunks)Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute/Helpers/ErrorHandler.cs(1 hunks)Frends.Template/Frends.Echo.Execute/Helpers/ErrorHandler.cs(0 hunks)Frends.Template/Frends.Echo.Execute/README.md(2 hunks)Frends.Template/setup.cmd(0 hunks)Frends.Template/setup.sh(0 hunks)FrendsTaskTemplate.csproj(1 hunks)README.md(1 hunks)
💤 Files with no reviewable changes (6)
- Frends.Template/setup.sh
- Frends.Template/Apache-2.0
- Frends.Template/Frends.Echo.Execute.Tests/Frends.Echo.Execute.Tests.csproj
- Frends.Template/Frends.Echo.Execute.Tests/GlobalSuppressions.cs
- Frends.Template/setup.cmd
- Frends.Template/Frends.Echo.Execute/Helpers/ErrorHandler.cs
🧰 Additional context used
📓 Path-based instructions (1)
Frends.*/**/*.cs
⚙️ CodeRabbit configuration file
Frends.*/**/*.cs: Code must follow Microsoft C# coding standards, including:
- PascalCase for public members and task parameters
- Proper naming for abbreviations (Csv, Url, Api)
- Use of var only when type is obvious
- Clean structure and no unused code
Files:
Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute/Definitions/Options.csFrends.Template/Frends.Echo.Execute/Frends.Echo.Execute/Definitions/Result.csFrends.Template/Frends.Echo.Execute/Frends.Echo.Execute/Frends.Echo.Execute.csFrends.Template/Frends.Echo.Execute/Frends.Echo.Execute/GlobalSuppressions.csFrends.Template/Frends.Echo.Execute/Frends.Echo.Execute.Tests/GlobalSuppressions.csFrends.Template/Frends.Echo.Execute/Frends.Echo.Execute/Helpers/ErrorHandler.cs
🧬 Code graph analysis (1)
Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute/Helpers/ErrorHandler.cs (2)
Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute/Definitions/Result.cs (1)
Result(6-25)Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute/Definitions/Error.cs (1)
Error(8-22)
🪛 markdownlint-cli2 (0.18.1)
README.md
45-45: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (29)
.gitignore (1)
470-471: Rider IDE folder addition looks good.Standard practice to exclude
.idea/directory from version control.Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute/Definitions/Result.cs (1)
25-25: Trailing newline addition is minor cosmetic improvement.No functional changes to the Result class. Documentation and structure are solid.
Frends.Template/.github/workflows/Execute_release.yml (1)
2-3: Permissions block correctly specifies write access for release workflow.Appropriate for a release workflow that may interact with repository contents. Consistent with broader permission hardening across workflows in this PR.
Frends.Template/.github/workflows/Execute_test_on_main.yml (1)
1-3: Workflow rename and read-only permissions are appropriate.Rename from "Execute_build_main" to "Execute_test_on_main" clarifies intent. Read-only contents permission is correct for a test workflow. Consistent with other workflow updates in this PR.
Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute/Definitions/Options.cs (1)
22-24: XML documentation example corrected to reflect actual default value.Updated from
falsetotrueto match the DefaultValue attribute and property initialization on line 24. Documentation now accurately represents the code behavior.Frends.Template/.github/workflows/Execute_test_on_push.yml (1)
1-3: Workflow rename and read-only permissions are consistent with other test workflows.Renamed from "Execute_build_test" to "Execute_test_on_push" for clarity. Permissions correctly limited to read-only. Aligns with the parallel Execute_test_on_main.yml workflow.
.github/workflows/BuildMaster.yml (2)
2-3: Permissions block correctly set to read-only.Appropriate for a workflow that packs and pushes packages without modifying repository contents directly.
18-18: Action version upgrades verified; .NET 8.0 migration requires validation.actions/checkout v6.0.1 was released Dec 2, 2025 and actions/setup-dotnet v5.0.0 was released Sep 3, 2025, both are current stable versions. v6 requires minimum Actions Runner version v2.329.0 for Docker container action scenarios.
However, ensure your team confirms:
- All GitHub-hosted runners meet the minimum runner version requirements.
- The codebase has been fully tested and verified against .NET 8.0 (migrating from 6.0.x is a significant jump).
- Any project files (.csproj) have been updated to target .NET 8.0.
Also applies to: 23-23, 25-25
.github/workflows/AutoBuild.yml (2)
2-3: Permissions block correctly set to read-only.Appropriate for a build workflow pushing to test feed without modifying repository.
18-18: Action version upgrades verified; .NET 8.0 migration requires validation (mirrors BuildMaster.yml).actions/checkout v6.0.1 was released Dec 2, 2025 and actions/setup-dotnet v5.0.0 was released Sep 3, 2025. Same upgrade pattern as BuildMaster.yml.
Same verification needed:
- Confirm GitHub-hosted runners meet minimum version v2.329.0 requirements.
- Validate full codebase compatibility with .NET 8.0.
- Confirm all .csproj files target .NET 8.0 (this should align with PR objectives, but requires verification).
Also applies to: 23-23, 25-25
FrendsTaskTemplate.csproj (1)
5-5: LGTM!The version bump from 1.4.0 to 1.5.0 is appropriate for this template refactoring PR.
Frends.Template/Frends.Echo.Execute/README.md (2)
3-3: LGTM! Template placeholder as intended.The
TaskDescriptionplaceholder is appropriate for a dotnet template and will be substituted during template instantiation.
33-34: Good documentation addition.The StyleCop.Analyzers version note helpfully explains the use of a beta version and its rationale.
Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute.sln (1)
13-14: LGTM! Workflow references updated correctly.The solution items now reference the renamed workflow files, maintaining consistency with the broader workflow restructuring.
Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute/Frends.Echo.Execute.cs (1)
16-16: Template placeholder as expected.The
TaskDescriptiontext serves as a clear placeholder for template instantiation.Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute/Helpers/ErrorHandler.cs (1)
8-23: LGTM! Clean error handling implementation.The error handler correctly:
- Preserves stack traces by using inner exceptions
- Supports custom error messages
- Returns structured error information in the Result object
- Follows good separation of concerns
Frends.Template/Frends.Echo.Execute/.gitignore (1)
1-421: LGTM! Comprehensive .gitignore.This is a well-structured .gitignore file covering all common .NET development artifacts, IDE files, and build outputs. The explicit preservation of
Directory.Build.rsp(line 99) is a good practice.Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute/GlobalSuppressions.cs (1)
5-9: LGTM! StyleCop suppressions updated appropriately.The suppressions are well-justified and align with Frends coding standards. The addition of SA1649 (FileNameMustMatchTypeName) suppression is appropriate for this template project.
Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute.Tests/GlobalSuppressions.cs (1)
3-7: LGTM! Appropriate test project suppressions.The StyleCop suppressions are reasonable for a test project, where documentation requirements are typically relaxed. The justifications are clear and consistent with Frends standards.
Frends.Template/.template.config/template.json (4)
8-9: Update identity and shortName for consistency. Template identity and short name are updated to reflect the new naming convention. No functional impact.
16-28: FullTaskName and Description symbols properly configured. Both parameter symbols are correctly defined with proper descriptions and default values. The isRequired flag on FullTaskName ensures users provide the mandatory input.
42-62: Derived symbols and regex patterns are correct. The three symbols properly extract Company, System, and Action names from the FullTaskName using well-formed regex patterns and corresponding forms. The regex captures are accurate for the documented format.
80-96: Forms section with regex transforms is well-structured. The regex patterns correctly define the transformation logic for extracting Company, System, and Action components from the dot-delimited FullTaskName.Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute.Tests/Frends.Echo.Execute.Tests.csproj (3)
1-6: Test project configuration is correct. Target framework, packability, and nullable settings are all appropriate for a test project.
8-10: Project reference path is correct. The relative path properly references the main project from the subfolder structure.
12-17: Test package references are appropriate. The mix of exact and wildcard versions provides flexibility while maintaining consistency with the main project's StyleCop version. Consider pinning exact versions in production templates for reproducibility, but this approach is acceptable here.Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute/Frends.Echo.Execute.csproj (3)
13-13: Description placeholder and Nullable setting are correct. The Description uses a template placeholder that will be replaced, and Nullable=disable is consistent across the project structure.Also applies to: 16-16
20-27: Verify analyzer package versions and availability. BothStyleCop.Analyzers(1.2.0-beta.556, a beta version) andFrendsTaskAnalyzers(1.*) need verification for availability and correctness. Confirm that FrendsTaskAnalyzers is available in the Frends NuGet feed specified in README.
30-34: All referenced files exist at the correct locations for the relative paths specified. The../CHANGELOG.mdpath on line 32 correctly resolves toFrends.Template/Frends.Echo.Execute/CHANGELOG.md, andmigration.jsonandFrendsTaskMetadata.jsonare both in the csproj directory. No path corrections are needed.
Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute/Frends.Echo.Execute.cs
Outdated
Show resolved
Hide resolved
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
README.md (1)
78-78: Update uninstall command to use new template shortName.The uninstall command on line 78 still references the old template name
frendstasktemplate. For consistency with the other updated commands in this file, update it to match the newfrends-taskshortName.🔎 Proposed fix
-`dotnet new uninstall frendstasktemplate` +`dotnet new uninstall frends-task`Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute/Frends.Echo.Execute.cs (3)
15-23: Replace placeholder documentation with actual task description and add<example>tag.The
<summary>contains a generic placeholder "TaskDescription" instead of a meaningful description. Additionally, the XML documentation is missing the required<example>tag.As per coding guidelines and PR objectives, public methods must include both
<summary>and<example>XML comments with proper content.🔎 Suggested documentation structure
/// <summary> - /// TaskDescription + /// Echoes the input string the specified number of times with a configurable delimiter. /// [Documentation](https://tasks.frends.com/tasks/frends-tasks/Frends-Echo-Execute) /// </summary> /// <param name="input">Essential parameters.</param> /// <param name="connection">Connection parameters.</param> /// <param name="options">Additional parameters.</param> /// <param name="cancellationToken">A cancellation token provided by Frends Platform.</param> /// <returns>object { bool Success, string Output, object Error { string Message, Exception AdditionalInfo } }</returns> + /// <example> + /// <code> + /// var input = new Input { Content = "Hello", Repeat = 3 }; + /// var connection = new Connection { ConnectionString = "" }; + /// var options = new Options { Delimiter = ", ", ThrowErrorOnFailure = false }; + /// var result = Echo.Execute(input, connection, options, CancellationToken.None); + /// // result.Output: "Hello, Hello, Hello" + /// </code> + /// </example>
24-24: Address or track the TODO comment about Connection parameter.The TODO suggests removing the Connection parameter if the task doesn't make connections. Currently, the connection parameter is accessed but not used meaningfully (line 34).
Consider either:
- Removing the Connection parameter if not needed for this task
- Implementing actual connection logic if required
- Creating a tracking issue if this will be addressed later
Do you want me to open a new issue to track this task?
33-34: Replace placeholder connection code with actual implementation or remove.The TODO comment and discard operator indicate this is placeholder code. The connection parameter is accessed but not used meaningfully.
This should be addressed consistently with the TODO on line 24 regarding the Connection parameter.
♻️ Duplicate comments (1)
README.md (1)
45-45: Add language identifier to fenced code block.This issue was previously flagged but remains unfixed. The code block on line 45 is missing a language specifier, which helps with syntax highlighting and accessibility. Update the opening fence to
shell.🔎 Proposed fix
-``` +```shell
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute/Frends.Echo.Execute.cs(1 hunks)Frends.Template/Frends.Echo.Execute/README.md(2 hunks)FrendsTaskTemplate.csproj(2 hunks)README.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- Frends.Template/Frends.Echo.Execute/README.md
- FrendsTaskTemplate.csproj
🧰 Additional context used
📓 Path-based instructions (1)
Frends.*/**/*.cs
⚙️ CodeRabbit configuration file
Frends.*/**/*.cs: Code must follow Microsoft C# coding standards, including:
- PascalCase for public members and task parameters
- Proper naming for abbreviations (Csv, Url, Api)
- Use of var only when type is obvious
- Clean structure and no unused code
Files:
Frends.Template/Frends.Echo.Execute/Frends.Echo.Execute/Frends.Echo.Execute.cs
🪛 markdownlint-cli2 (0.18.1)
README.md
45-45: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[]# Frends Task Pull Request
Summary
Review Checklist
1. Frends Task Project Files
Frends.*/Frends.*/*.csproj<PackageLicenseExpression>MIT</PackageLicenseExpression>)<Version><Authors>Frends</Authors><Description><RepositoryUrl><GenerateDocumentationFile>true</GenerateDocumentationFile>2. File: FrendsTaskMetadata.json
Frends.*/Frends.*/FrendsTaskMetadata.json3. File: README.md
Frends.*/README.md4. File: CHANGELOG.md
Frends.*/CHANGELOG.md5. File: migration.json
Frends.*/Frends.*/migration.json6. Source Code Documentation
Frends.*/Frends.*/*.cs<summary>XML comments<example>XML comments<frendsdocs>XML comments, if needed7. GitHub Actions Workflows
.github/workflows/*.yml*_test.yml*_main.yml*_release.yml8. Task Result Object Structure
Frends.*/Frends.*/*.csSuccess(bool)Additional Notes
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.