chore: Dependency version bumps, editorconfig fixes, and repo config updates#7
chore: Dependency version bumps, editorconfig fixes, and repo config updates#7kploch wants to merge 2 commits into
Conversation
…ops, and fix editorconfig Bump all centralised package versions to latest stable releases: - Microsoft.Extensions/EF Core 10.0.1 → 10.0.5 - xUnit v3 3.2.0 → 3.2.2, Test.Sdk 18.0.1 → 18.3.0 - Coverlet 6.0.4 → 8.0.0, Roslynator 4.14.1 → 4.15.0 - SonarAnalyzer 10.16.0 → 10.21.0, and others Expand Ploch.Packages.props with a centralised PlochPackagesVersion property (3.0.0) and add all commonly referenced Ploch packages. Additional fixes: - Remove SA1502 enforcement from StyleCop editorconfig - Add ReSharper XML indent settings and XML/props indent rules - Add AI tool config files (.cursorrules, AGENTS.md, etc.) to gitignore - Add .syncignore, ReSharper file layout, Claude Code context menu script - Add editorconfig hierarchy sample project and editorconfig files - Add development notes and guidelines - Update sample solution README - Remove obsolete .junie/guidelines.md Refs: #6 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
📝 WalkthroughWalkthroughBatch maintenance: added ignore/config files, removed an obsolete guideline, introduced EditorConfig samples and ReSharper layout, and bumped many centralized NuGet analyzer/test/framework and Ploch package versions; also added small docs and a Windows context-menu registry script. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Review Summary by QodoUpdate dependencies, expand Ploch packages, fix editorconfig, and add development documentation
WalkthroughsDescription• Bump all centralised package versions to latest stable releases - Microsoft.Extensions/EF Core 10.0.1 → 10.0.5 - xUnit v3 3.2.0 → 3.2.2, Test.Sdk 18.0.1 → 18.3.0 - Coverlet 6.0.4 → 8.0.0, Roslynator 4.14.1 → 4.15.0 - SonarAnalyzer 10.16.0 → 10.21.0, and others • Expand Ploch.Packages.props with centralised PlochPackagesVersion property (3.0.0) and add all commonly referenced Ploch packages • Fix editorconfig: remove SA1502 enforcement, add ReSharper XML indent settings, add XML/props file indent rules • Add development documentation, repository configuration files, and editorconfig hierarchy samples Diagramflowchart LR
A["Dependency Versions"] -->|Update to latest| B["Package Props Files"]
C["Ploch.Packages.props"] -->|Add PlochPackagesVersion| D["Centralised Version Management"]
E["EditorConfig"] -->|Remove SA1502, Add XML rules| F["Enhanced Code Style"]
G["New Files"] -->|Add development docs & configs| H["Repository Configuration"]
B --> I["Updated Solution"]
D --> I
F --> I
H --> I
File Changes1. dependencies/Analyzers.Global.Packages.props
|
Code Review by Qodo
1. Single PlochPackagesVersion used
|
![codereviewbot-ai[bot]](https://avatars.githubusercontent.com/in/309638?s=60&v=4){kind=small}
| @@ -0,0 +1,4 @@ | |||
| namespace Ploch.EditorConfig.FormattingCheck | |||
| { | |||
| public interface NewInterface { } | |||
There was a problem hiding this comment.
The interface NewInterface is currently empty. If this is intended as a marker interface, ensure that its usage is justified by the design. Otherwise, consider removing or implementing members to provide meaningful functionality.
There was a problem hiding this comment.
False positive. This is a sample file in editor-config/editorconfig-hierarchy/ used to test EditorConfig hierarchy behaviour. It is not production code — it exists solely to verify that editorconfig rules are applied correctly at different directory levels.
codeant-ai
Bot
added
the
size:XXL
|
The interface |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates package dependencies to newer versions, adds documentation for coding guidelines and analyzers, and introduces ReSharper file layout configurations. It also includes a Windows registry script for Claude Code integration and sample projects for testing editor configurations. Key feedback includes addressing hardcoded local paths in the registry script, generalizing specific ignore patterns in the sync configuration, removing UTF-8 BOMs for better cross-platform support, and cleaning up obsolete TODO comments.
| ; Add "Open with Claude Code" to folder context menu | ||
| [HKEY_CLASSES_ROOT\Directory\shell\OpenWithClaudeCode] | ||
| @="Open with Claude Code" | ||
| "Icon"="C:\\DevNet\\github\\open-with-claude-code\\claude.ico,0" |
There was a problem hiding this comment.
The icon path is hardcoded to a local directory: C:\\DevNet\\github\\open-with-claude-code\\claude.ico,0. This path is specific to your machine and will not work for other users. The same issue exists on line 15.
For a script in a shared repository, it's crucial to avoid local, absolute paths. Consider using a placeholder that a setup script can replace, or referencing a path via a system-wide environment variable.
There was a problem hiding this comment.
Addressed in commit dcd3e77. Fixed cmd.exe quoting (removed extra outer quotes) and added a comment about the hardcoded icon path.
| /.idea/**/.idea/dataSources.xml | ||
| /.idea/**/.idea/inspectionProfiles/Project_Default.xml | ||
| /.idea/.idea.Ploch.Common.Data/.idea/vcs.xml |
There was a problem hiding this comment.
These lines contain paths that are either incorrect or too specific for a general-purpose ignore file.
- Lines 391 and 392 reference a nested
.ideadirectory (.idea/**/.idea), which is an unlikely structure. - Line 393 contains a hardcoded project name (
Ploch.Common.Data), which makes the rule not portable across different projects.
It's recommended to remove these lines. Broader, more generic patterns (like **/.idea/) are preferred.
There was a problem hiding this comment.
Addressed in commit dcd3e77. Replaced project-specific .idea paths with generic glob patterns.
| @@ -0,0 +1,9 @@ | |||
| <Project Sdk="Microsoft.NET.Sdk"> | |||
There was a problem hiding this comment.
This file starts with a UTF-8 Byte Order Mark (BOM), which can cause issues with some tools and platforms, especially outside of Windows. It's generally recommended to use UTF-8 without a BOM for text files in a cross-platform project. You can enforce this by adding charset = utf-8 to your .editorconfig file.
<Project Sdk="Microsoft.NET.Sdk">
There was a problem hiding this comment.
False positive. UTF-8 BOM is standard for .NET SDK project files generated by Visual Studio and dotnet new. This is a sample project file for testing editorconfig hierarchy, not a cross-platform script.
| resharper_built_in_type_reference_style_for_member_access_highlighting = warning | ||
| resharper_built_in_type_reference_style_highlighting = hint | ||
|
|
||
| resharper_c_sharp_warnings_cs8618_cs9264_highlighting = error # TODO: Check this: 'c_sharp' vs 'csharp' (it's correct: https://www.jetbrains.com/help/resharper/Reference__Code_Inspections_CSHARP.html#BestPractice) |
There was a problem hiding this comment.
This line contains a TODO comment. It appears the check has been completed and the comment confirms the setting is correct. The TODO and the explanatory comment should be removed to keep the configuration file clean.
resharper_c_sharp_warnings_cs8618_cs9264_highlighting = error
There was a problem hiding this comment.
Addressed in commit dcd3e77. Removed the TODO prefix — the investigation confirmed it is correct.
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| <PackageVersion Include="IDisposableAnalyzers" Version="4.0.8" /> | ||
| <PackageVersion Include="JetBrains.Annotations" Version="2025.2.4" /> | ||
| <PackageVersion Include="Microsoft.AspNetCore.Mvc.ViewFeatures" Version="2.3.0" /> | ||
| <PackageVersion Include="Microsoft.AspNetCore.Mvc.ViewFeatures" Version="2.3.9" /> |
There was a problem hiding this comment.
Suggestion: The package version 2.3.9 for Microsoft.AspNetCore.Mvc.ViewFeatures does not correspond to a valid ASP.NET Core package version (the 2.x line ended at 2.2.x), so using it in central package management will cause NuGet restore to fail for any project that references this package version. [possible bug]
Severity Level: Major ⚠️
- ❌ Consuming projects cannot restore when using ViewFeatures package.
- ⚠️ Shared central package config causes unexpected build failures.| <PackageVersion Include="Microsoft.AspNetCore.Mvc.ViewFeatures" Version="2.3.9" /> | |
| <PackageVersion Include="Microsoft.AspNetCore.Mvc.ViewFeatures" Version="2.2.0" /> |
Steps of Reproduction ✅
1. Use this repository as intended (per PR description: shared centralised package version
definitions) and include `dependencies/Common.Packages.props` in a consuming .NET solution
for central package management.
2. In that consuming solution, add a project with a `<PackageReference
Include="Microsoft.AspNetCore.Mvc.ViewFeatures" />` that omits the `Version` attribute so
it uses the central version from `dependencies/Common.Packages.props:23`.
3. Run `dotnet restore` for the consuming solution; NuGet resolves
`Microsoft.AspNetCore.Mvc.ViewFeatures` using the centrally defined version `2.3.9` from
`dependencies/Common.Packages.props` line 23.
4. Observe NuGet restore failing because package `Microsoft.AspNetCore.Mvc.ViewFeatures`
has no published `2.3.9` version in the 2.x line (which ended at 2.2.x), so the consuming
project cannot restore or build.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** dependencies/Common.Packages.props
**Line:** 23:23
**Comment:**
*Possible Bug: The package version `2.3.9` for `Microsoft.AspNetCore.Mvc.ViewFeatures` does not correspond to a valid ASP.NET Core package version (the 2.x line ended at 2.2.x), so using it in central package management will cause NuGet restore to fail for any project that references this package version.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.There was a problem hiding this comment.
False positive. Version 2.3.9 does exist on NuGet.org — verified via the NuGet API (https://api.nuget.org/v3-flatcontainer/microsoft.aspnetcore.mvc.viewfeatures/index.json lists it). The 2.x ASP.NET Core packages continued receiving security patches beyond 2.2.x.
| resharper_arrange_type_member_modifiers_highlighting = hint | ||
| resharper_arrange_type_modifiers_highlighting = hint | ||
|
|
||
| resharper_async_apostle_configure_await_highlighting_highlighting = hint |
There was a problem hiding this comment.
Suggestion: The ReSharper setting key for the async apostle configure-await inspection has a duplicated "_highlighting" suffix, so ReSharper will not recognize this option and the intended severity will not be applied, leading to unexpected default behavior for this inspection. [logic error]
Severity Level: Major ⚠️
- ⚠️ Async apostle inspection ignores configured hint severity.
- ⚠️ ReSharper users see unexpected defaults for configure-await rule.| resharper_async_apostle_configure_await_highlighting_highlighting = hint | |
| resharper_async_apostle_configure_await_highlighting = hint |
Steps of Reproduction ✅
1. Open the shared configuration file
`editor-config/editorconfig-hierarchy/library-repository/using-shared-root-file.editorconfig`
and locate the ReSharper settings block around line 519, where
`resharper_async_apostle_configure_await_highlighting_highlighting = hint` is defined
(verified via Read tool).
2. In the same file, compare this key with other ReSharper severity keys such as
`resharper_built_in_type_reference_style_for_member_access_highlighting` at lines 135–136
and many other `resharper_*_highlighting` entries, all of which end with a single
`_highlighting` suffix (verified via Read on the same file).
3. Note that the async apostle entry is the only one using the duplicated
`_highlighting_highlighting` suffix; a global Grep search for
`resharper_async_apostle_configure_await_highlighting =` in the repository returns no
matches, while several files (including this one) contain the double-suffix variant,
indicating a consistent typo rather than a different supported key (verified via Grep on
`/workspace/mrploch-development`).
4. When a C# project uses this editorconfig file as part of its configuration (e.g., by
copying it to the repository root as `.editorconfig` for a library project), ReSharper
will read severity settings from keys that match known inspection IDs such as
`resharper_async_apostle_configure_await_highlighting`; because the key at line 519 is
mis-typed, ReSharper will ignore this line and the async apostle "configure await"
inspection will fall back to its default severity instead of the intended `hint`, making
severity configuration for this rule ineffective.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** editor-config/editorconfig-hierarchy/library-repository/using-shared-root-file.editorconfig
**Line:** 519:519
**Comment:**
*Logic Error: The ReSharper setting key for the async apostle configure-await inspection has a duplicated `"_highlighting"` suffix, so ReSharper will not recognize this option and the intended severity will not be applied, leading to unexpected default behavior for this inspection.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.There was a problem hiding this comment.
Addressed in commit dcd3e77. Removed the duplicated _highlighting suffix.
| <TypePattern DisplayName="xUnit.net Test Classes" RemoveRegions="All"> | ||
| <TypePattern.Match> | ||
| <And> | ||
| <Kind Is="Class" /> | ||
| <HasMember> | ||
| <And> | ||
| <Kind Is="Method" /> | ||
| <HasAttribute Inherited="True" Name="Xunit.FactAttribute" /> | ||
| <HasAttribute Inherited="True" Name="Xunit.TheoryAttribute" /> |
There was a problem hiding this comment.
Suggestion: The xUnit test class detection currently requires a method that has both Xunit.FactAttribute and Xunit.TheoryAttribute simultaneously, which effectively never happens in real code, so the "xUnit.net Test Classes" pattern will almost never match and the custom layout will not be applied to xUnit test classes; wrapping these attributes in an <Or> condition fixes the logic. [logic error]
Severity Level: Major ⚠️
- ⚠️ xUnit test classes not recognized by ReSharper layout.
- ⚠️ xUnit cleanup uses generic default member ordering.| <TypePattern DisplayName="xUnit.net Test Classes" RemoveRegions="All"> | |
| <TypePattern.Match> | |
| <And> | |
| <Kind Is="Class" /> | |
| <HasMember> | |
| <And> | |
| <Kind Is="Method" /> | |
| <HasAttribute Inherited="True" Name="Xunit.FactAttribute" /> | |
| <HasAttribute Inherited="True" Name="Xunit.TheoryAttribute" /> | |
| <TypePattern DisplayName="xUnit.net Test Classes" RemoveRegions="All"> | |
| <TypePattern.Match> | |
| <And> | |
| <Kind Is="Class" /> | |
| <HasMember> | |
| <And> | |
| <Kind Is="Method" /> | |
| <Or> | |
| <HasAttribute Inherited="True" Name="Xunit.FactAttribute" /> | |
| <HasAttribute Inherited="True" Name="Xunit.TheoryAttribute" /> | |
| </Or> |
Steps of Reproduction ✅
1. Configure ReSharper in Visual Studio to use the file layout defined in
`repository-config/resharper/file-layout-current-visual-studio-resharper.xml` by importing
it as the current file layout (pattern "xUnit.net Test Classes" is defined at lines 18–74
in this file).
2. In any C# project using that configuration, add a typical xUnit test class (for example
`SampleTests`) where methods are annotated with either `[Fact]` or `[Theory]`, but not
both, which is normal xUnit usage.
3. Run ReSharper's "Reorder Type Members" or a Code Cleanup profile that applies the
imported file layout to the test class, causing ReSharper to evaluate the
`<TypePattern.Match>` for xUnit classes at lines 18–30.
4. Observe that the test class is not matched by the "xUnit.net Test Classes" pattern and
instead falls back to the "Default Pattern" at lines 126–215, because the current match
condition requires a single method to have both `Xunit.FactAttribute` and
`Xunit.TheoryAttribute` simultaneously, which never occurs in real xUnit code.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** repository-config/resharper/file-layout-current-visual-studio-resharper.xml
**Line:** 18:26
**Comment:**
*Logic Error: The xUnit test class detection currently requires a method that has both `Xunit.FactAttribute` and `Xunit.TheoryAttribute` simultaneously, which effectively never happens in real code, so the "xUnit.net Test Classes" pattern will almost never match and the custom layout will not be applied to xUnit test classes; wrapping these attributes in an `<Or>` condition fixes the logic.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.There was a problem hiding this comment.
Addressed in commit dcd3e77. Changed <And> to <Or> for Fact/Theory attribute matching.
| <HasAttribute Name="Xunit.FactAttribute" /> | ||
| <HasAttribute Name="Xunit.TheoryAttribute" /> |
There was a problem hiding this comment.
Suggestion: The xUnit test method entry similarly requires each method to have both Xunit.FactAttribute and Xunit.TheoryAttribute at the same time, so no normal test methods are matched and the intended ordering for xUnit tests is never applied; using an <Or> between the attributes corrects this. [logic error]
Severity Level: Major ⚠️
- ⚠️ xUnit test methods not grouped under test methods entry.
- ⚠️ xUnit file layout inconsistent across code cleanup runs.| <HasAttribute Name="Xunit.FactAttribute" /> | |
| <HasAttribute Name="Xunit.TheoryAttribute" /> | |
| <Or> | |
| <HasAttribute Name="Xunit.FactAttribute" /> | |
| <HasAttribute Name="Xunit.TheoryAttribute" /> | |
| </Or> |
Steps of Reproduction ✅
1. Import `repository-config/resharper/file-layout-current-visual-studio-resharper.xml`
into ReSharper as the current file layout, so the xUnit-specific "Test Methods" entry at
lines 62–73 is active for xUnit test classes.
2. In a project using xUnit, create a class that satisfies the surrounding "xUnit.net Test
Classes" type pattern (lines 18–30) by containing at least one method with `[Fact]` or
`[Theory]` (as normal), but where no method has both attributes.
3. Run ReSharper's "Reorder Type Members" or Code Cleanup using this layout so that
ReSharper evaluates the `<Entry.DisplayName="Test Methods">` block at lines 62–73 when
ordering members inside the test class.
4. Observe that none of the xUnit test methods are placed into the "Test Methods" group
and are instead ordered as generic members, because the current `<Entry.Match>` requires
each method to have both `Xunit.FactAttribute` and `Xunit.TheoryAttribute`, which real
xUnit methods never do.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** repository-config/resharper/file-layout-current-visual-studio-resharper.xml
**Line:** 66:67
**Comment:**
*Logic Error: The xUnit test method entry similarly requires each method to have both `Xunit.FactAttribute` and `Xunit.TheoryAttribute` at the same time, so no normal test methods are matched and the intended ordering for xUnit tests is never applied; using an `<Or>` between the attributes corrects this.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| <HasAttribute Name="NUnit.Framework.TestAttribute" /> | ||
| <HasAttribute Name="NUnit.Framework.TestCaseAttribute" /> | ||
| <HasAttribute Name="NUnit.Framework.TestCaseSourceAttribute" /> |
There was a problem hiding this comment.
Suggestion: The NUnit test fixture detection via HasMember requires a single method decorated with all of Test, TestCase, and TestCaseSource attributes simultaneously, which is unrealistic and prevents fixtures that rely only on test method attributes from being recognized; using an <Or> condition across these attributes matches the intended range of NUnit test methods. [logic error]
Severity Level: Major ⚠️
- ⚠️ NUnit fixtures detected only when class attributes used.
- ⚠️ NUnit fixtures without class attributes use default order.| <HasAttribute Name="NUnit.Framework.TestAttribute" /> | |
| <HasAttribute Name="NUnit.Framework.TestCaseAttribute" /> | |
| <HasAttribute Name="NUnit.Framework.TestCaseSourceAttribute" /> | |
| <Or> | |
| <HasAttribute Name="NUnit.Framework.TestAttribute" /> | |
| <HasAttribute Name="NUnit.Framework.TestCaseAttribute" /> | |
| <HasAttribute Name="NUnit.Framework.TestCaseSourceAttribute" /> | |
| </Or> |
Steps of Reproduction ✅
1. Import `repository-config/resharper/file-layout-current-visual-studio-resharper.xml`
into ReSharper as the active file layout so the "NUnit Test Fixtures" pattern at lines
75–125 controls ordering of NUnit fixtures.
2. In a C# project using NUnit, create a test class without `[TestFixture]` or
`[TestFixtureSource]` attributes but containing methods marked with `[Test]`,
`[TestCase]`, or `[TestCaseSource]`, which is a supported NUnit style relying on method
attributes only.
3. Run ReSharper's "Reorder Type Members" or Code Cleanup using the imported layout so
that the `<HasMember>` condition at lines 82–89 is evaluated to detect NUnit fixtures
based on their test methods.
4. Observe that the class is not detected as an "NUnit Test Fixture" and is ordered using
the generic "Default Pattern" at lines 126–215, because the current `<HasMember>`
definition requires a single method to have all three NUnit attributes (`Test`,
`TestCase`, and `TestCaseSource`) simultaneously, which does not occur in normal NUnit
usage.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** repository-config/resharper/file-layout-current-visual-studio-resharper.xml
**Line:** 85:87
**Comment:**
*Logic Error: The NUnit test fixture detection via `HasMember` requires a single method decorated with all of `Test`, `TestCase`, and `TestCaseSource` attributes simultaneously, which is unrealistic and prevents fixtures that rely only on test method attributes from being recognized; using an `<Or>` condition across these attributes matches the intended range of NUnit test methods.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.There was a problem hiding this comment.
Addressed in commit dcd3e77. Changed <And> to <Or> for NUnit Test/TestCase/TestCaseSource matching.
| <HasAttribute Name="NUnit.Framework.TestAttribute" /> | ||
| <HasAttribute Name="NUnit.Framework.TestCaseAttribute" /> | ||
| <HasAttribute Name="NUnit.Framework.TestCaseSourceAttribute" /> |
There was a problem hiding this comment.
Suggestion: The NUnit test methods entry requires each method to have all three NUnit attributes (Test, TestCase, and TestCaseSource) at once, which typical NUnit tests do not, so no methods are matched for the "Test Methods" group and the intended ordering is skipped; changing this to an <Or> condition aligns with real NUnit usage. [logic error]
Severity Level: Major ⚠️
- ⚠️ NUnit test methods never matched by test methods group.
- ⚠️ NUnit test ordering falls back to default layout.| <HasAttribute Name="NUnit.Framework.TestAttribute" /> | |
| <HasAttribute Name="NUnit.Framework.TestCaseAttribute" /> | |
| <HasAttribute Name="NUnit.Framework.TestCaseSourceAttribute" /> | |
| <Or> | |
| <HasAttribute Name="NUnit.Framework.TestAttribute" /> | |
| <HasAttribute Name="NUnit.Framework.TestCaseAttribute" /> | |
| <HasAttribute Name="NUnit.Framework.TestCaseSourceAttribute" /> | |
| </Or> |
Steps of Reproduction ✅
1. Import `repository-config/resharper/file-layout-current-visual-studio-resharper.xml`
into ReSharper so that the "NUnit Test Fixtures" block, including the "Test Methods" entry
at lines 112–124, is used for ordering NUnit classes.
2. In a project with NUnit tests, create or open a fixture class that is recognized by the
outer type pattern (for example via `[TestFixture]` at lines 80–81) and contains methods
with typical NUnit attributes such as `[Test]`, `[TestCase]`, or `[TestCaseSource]`, but
not all three on the same method.
3. Run ReSharper's "Reorder Type Members" or apply a Code Cleanup profile using this
layout so that the `<Entry.DisplayName="Test Methods">` block at lines 112–124 is
evaluated for each method in the fixture.
4. Observe that none of the NUnit test methods are placed into the "Test Methods" group
and are instead ordered as generic members, because the match currently requires a single
method to have `Test`, `TestCase`, and `TestCaseSource` attributes simultaneously, which
standard NUnit methods do not.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** repository-config/resharper/file-layout-current-visual-studio-resharper.xml
**Line:** 116:118
**Comment:**
*Logic Error: The NUnit test methods entry requires each method to have all three NUnit attributes (`Test`, `TestCase`, and `TestCaseSource`) at once, which typical NUnit tests do not, so no methods are matched for the "Test Methods" group and the intended ordering is skipped; changing this to an `<Or>` condition aligns with real NUnit usage.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| "Position"="Top" | ||
|
|
||
| [HKEY_CLASSES_ROOT\Directory\shell\OpenWithClaudeCode\command] | ||
| @="cmd.exe /c \"cd /d \"%1\" && claude\"" |
There was a problem hiding this comment.
Suggestion: The command for the folder context menu wraps the entire cd /d "%1" && claude sequence in an extra pair of quotes, so cmd.exe /c will misparse the nested quotes and the cd will fail for directories (especially those with spaces) instead of changing into the selected folder and launching Claude. [logic error]
Severity Level: Major ⚠️
- ❌ Folder context menu opens Claude in wrong directory.
- ⚠️ Developers cannot reliably open Claude in target folder.| @="cmd.exe /c \"cd /d \"%1\" && claude\"" | |
| @="cmd.exe /c cd /d \"%1\" && claude" |
Steps of Reproduction ✅
1. On a Windows machine, import the registry script
`/workspace/mrploch-development/scripts/add_context_menu.reg` so that the key
`[HKEY_CLASSES_ROOT\Directory\shell\OpenWithClaudeCode\command]` with value on line 10 is
registered.
2. In File Explorer, right‑click any folder whose path contains a space (e.g., `C:\DevNet
Projects`) and choose the "Open with Claude Code" entry added by lines 3–7 of
`/workspace/mrploch-development/scripts/add_context_menu.reg`.
3. Explorer launches `cmd.exe` with the command string from line 10: `cmd.exe /c "cd /d
"<folder-path>" && claude"`, where the outer quotes wrap the entire `cd /d "%1" && claude`
sequence.
4. Because of the nested quotes, `cmd.exe` ends the quoted command at the second quote
(after `cd /d `), so `cd` executes without the folder path and does not change into the
selected directory; Claude (if launched) starts in the default working directory instead
of the folder the user clicked.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** scripts/add_context_menu.reg
**Line:** 10:10
**Comment:**
*Logic Error: The command for the folder context menu wraps the entire `cd /d "%1" && claude` sequence in an extra pair of quotes, so `cmd.exe /c` will misparse the nested quotes and the `cd` will fail for directories (especially those with spaces) instead of changing into the selected folder and launching Claude.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.There was a problem hiding this comment.
Addressed in commit dcd3e77. Fixed quoting and changed %v to %V.
| "Position"="Top" | ||
|
|
||
| [HKEY_CLASSES_ROOT\Directory\Background\shell\OpenWithClaudeCode\command] | ||
| @="cmd.exe /c \"cd /d \"%v\" && claude\"" |
There was a problem hiding this comment.
Suggestion: The background context menu command both uses the wrong placeholder (%v instead of the standard %V) and adds an extra outer pair of quotes around the cd /d "%V" && claude sequence, so cmd.exe will not receive the correct folder path and the cd will fail instead of opening Claude in the clicked directory. [logic error]
Severity Level: Major ⚠️
- ❌ Background context menu opens Claude in incorrect directory.
- ⚠️ Background entry may not receive correct folder path.| @="cmd.exe /c \"cd /d \"%v\" && claude\"" | |
| @="cmd.exe /c cd /d \"%V\" && claude" |
Steps of Reproduction ✅
1. On a Windows machine, import
`/workspace/mrploch-development/scripts/add_context_menu.reg` so that
`[HKEY_CLASSES_ROOT\Directory\Background\shell\OpenWithClaudeCode\command]` with the value
from line 19 is registered.
2. In File Explorer, open any folder whose path contains a space, right‑click on the empty
background area (not on a file/folder), and choose "Open with Claude Code" added by lines
12–16 of `scripts/add_context_menu.reg`.
3. Explorer invokes `cmd.exe` with the command from line 19: `cmd.exe /c "cd /d
"<folder-path-or-%v-expansion>" && claude"`, where the entire `cd /d "%v" && claude`
sequence is wrapped in outer quotes.
4. The nested quotes cause `cmd.exe` to terminate the quoted command after `cd /d `, so
`cd` executes without the intended directory argument; as a result, Claude starts in the
default working directory instead of the folder whose background was right‑clicked, and
using `%v` instead of the standard `%V` placeholder further risks incorrect or empty
folder path expansion in this background context.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** scripts/add_context_menu.reg
**Line:** 19:19
**Comment:**
*Logic Error: The background context menu command both uses the wrong placeholder (`%v` instead of the standard `%V`) and adds an extra outer pair of quotes around the `cd /d "%V" && claude` sequence, so `cmd.exe` will not receive the correct folder path and the `cd` will fail instead of opening Claude in the clicked directory.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
|
CodeAnt AI finished reviewing your PR. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9e35108a3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <HasAttribute Inherited="True" Name="Xunit.FactAttribute" /> | ||
| <HasAttribute Inherited="True" Name="Xunit.TheoryAttribute" /> |
There was a problem hiding this comment.
The xUnit pattern currently requires a single method to have both Xunit.FactAttribute and Xunit.TheoryAttribute because both checks are inside the same <And>. In normal xUnit code, methods are annotated with one or the other, so this pattern will fail to classify most test classes/methods and the dedicated test-member ordering rules will not run.
Useful? React with 👍 / 👎.
| <HasAttribute Name="NUnit.Framework.TestAttribute" /> | ||
| <HasAttribute Name="NUnit.Framework.TestCaseAttribute" /> | ||
| <HasAttribute Name="NUnit.Framework.TestCaseSourceAttribute" /> |
There was a problem hiding this comment.
The NUnit test-method matcher also uses <And> across TestAttribute, TestCaseAttribute, and TestCaseSourceAttribute, which means a method must carry all three attributes to be recognized as a test. Typical NUnit methods use only one of these attributes, so they will be misclassified as regular members and won’t receive the intended test-method ordering (the same issue appears in the class HasMember branch).
Useful? React with 👍 / 👎.
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| <PropertyGroup> | ||
| <PlochPackagesVersion>3.0.0</PlochPackagesVersion> | ||
| </PropertyGroup> | ||
| <ItemGroup> | ||
| <PackageVersion Include="Ploch.Data.GenericRepository" Version="2.1.0" /> | ||
| <PackageVersion Include="Ploch.Data.GenericRepository.EFCore" Version="2.1.0" /> | ||
| <PackageVersion Include="Ploch.Data.Model" Version="2.1.0" /> | ||
| <PackageVersion Include="Ploch.Common.AppServices" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.Data.GenericRepository" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.Data.GenericRepository.EFCore" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.Data.Model" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.Data.EFCore" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.Data.EFCore.IntegrationTesting" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.Data.EFCore.SqLite" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.Data.EFCore.SqlServer" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.Data.GenericRepository.EFCore.IntegrationTesting" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.Common" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.Common.DependencyInjection" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.Common.Net9" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.Common.DataAnnotations" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.TestingSupport" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.TestingSupport.Xunit" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.TestingSupport.FluentAssertions" Version="$(PlochPackagesVersion)" /> |
There was a problem hiding this comment.
1. Single plochpackagesversion used 📎 Requirement gap ⛯ Reliability
dependencies/Ploch.Packages.props still uses a single PlochPackagesVersion property for both Ploch.Common*/Ploch.TestingSupport* and Ploch.Data* packages. This violates the required split into separate Common/Data version properties and can reintroduce restore/build failures when the repos release independently.
Agent Prompt
## Issue description
`dependencies/Ploch.Packages.props` uses a single `PlochPackagesVersion` for all Ploch packages, but compliance requires separate version properties for Common and Data packages.
## Issue Context
Ploch Common and Data packages release independently; using one pinned version can cause missing-version restores (NU1603) and build failures in consuming repos when warnings are treated as errors.
## Fix Focus Areas
- dependencies/Ploch.Packages.props[2-21]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Acknowledged. This is tracked by issue #5 and will be addressed in a follow-up PR that splits into per-repo version variables.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (4)
development-notes-guidelines/findings.md (1)
5-5: Tighten wording for clarity/professional tone.Consider replacing “stuff I found” with more precise language to keep this notes set consistent and searchable.
Suggested wording
-This document contains stuff I found and might want to review later. +This document captures items I discovered that may require follow-up review.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@development-notes-guidelines/findings.md` at line 5, Replace the casual phrase "stuff I found" in the document title/sentence with a clearer, professional phrase such as "findings and observations", "notes and findings", or "identified findings" to improve searchability and tone; update the sentence "This document contains stuff I found and might want to review later." to a more precise variant like "This document contains findings and observations to review later." and ensure similar phrasing is used consistently throughout the file.development-notes-guidelines/coding-guidelines-notes.md (1)
3-3: Use conventional “C#” naming in the section header.This improves readability and aligns with common .NET documentation terminology.
Suggested heading update
-## CSharpCodingGuidelines Site +## C# Coding Guidelines Site🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@development-notes-guidelines/coding-guidelines-notes.md` at line 3, Replace the nonstandard header "CSharpCodingGuidelines Site" with the conventional C# naming by updating that section heading to "C# Coding Guidelines" (or "C# Coding Guidelines Site" if the word "Site" must be preserved) so the header uses the "C#" token; locate the header text "CSharpCodingGuidelines Site" and change it accordingly.development-notes-guidelines/net-analyzers.md (1)
9-20: Consider using labeled Markdown links instead of raw URLs.The list is useful, but link labels (package/repo names) will make scanning and future maintenance easier.
Suggested doc refactor
-- https://www.nuget.org/packages/Philips.CodeAnalysis.MoqAnalyzers +- [Philips.CodeAnalysis.MoqAnalyzers](https://www.nuget.org/packages/Philips.CodeAnalysis.MoqAnalyzers) ... -- https://github.com/GuOrg/Gu.Analyzers +- [Gu.Analyzers (GitHub)](https://github.com/GuOrg/Gu.Analyzers)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@development-notes-guidelines/net-analyzers.md` around lines 9 - 20, Replace the raw URLs in the list with labeled Markdown links using the package or repo names as the link text (e.g., "Philips.CodeAnalysis.MoqAnalyzers", "ErrorProne.NET.CoreAnalyzers", "IDisposableAnalyzers", "Agoda.Analyzers", "SmartAnalyzers.CSharpExtensions.Annotations", "Philips.CodeAnalysis.MaintainabilityAnalyzers", "ReflectionAnalyzers", "StructuredLogging.Analyzers", "Spectre.Console.Analyzer", "SerilogAnalyzer", "TestableIO.System.IO.Abstractions.Analyzers", and "Gu.Analyzers") so each list item reads like "PackageName — [PackageName](URL)"; keep the same URLs but use the package/repo name as the label for readability and easier maintenance.scripts/add_context_menu.reg (1)
6-7: Avoid machine-specific icon paths in shared registry script.The
Iconvalue at Line 6 and Line 15 hardcodes a local path (C:\DevNet\...) that won’t exist for most contributors.🔧 Proposed fix
-"Icon"="C:\\DevNet\\github\\open-with-claude-code\\claude.ico,0" +; Optional: omit icon to avoid machine-specific paths +; "Icon"="C:\\Path\\To\\claude.ico,0"Also applies to: 15-16
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/add_context_menu.reg` around lines 6 - 7, The registry script hardcodes a machine-specific Icon path ("Icon"="C:\\DevNet\\github\\open-with-claude-code\\claude.ico,0"); update the "Icon" entries referencing claude.ico to use a relocatable approach: remove the absolute C:\\DevNet path and either (a) use an environment-variable-based path such as "%ProgramFiles%\\open-with-claude-code\\claude.ico,0" and store the value as an expanded string (REG_EXPAND_SZ / hex(2)) in the .reg, or (b) omit the Icon entries and document that installers must place claude.ico into a known install folder and set the Icon at install time; change both "Icon" occurrences accordingly so the .reg no longer contains a local-dev absolute path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dependencies/Ploch.Packages.props`:
- Around line 3-16: The sample-solution local packages file is overriding the
centralized PlochPackagesVersion by pinning Ploch.Common to 2.0.1; update the
sample-solution's directory.packages.props so it no longer pins a conflicting
version—either remove the explicit Ploch.Common entry or change it to reference
the centralized variable/3.0.0 (i.e., align it with PlochPackagesVersion used in
Ploch.Packages.props) to ensure the central version (PlochPackagesVersion /
Ploch.Common) is honored across the repo.
In `@dependencies/Testing.Packages.props`:
- Around line 42-45: This central package list includes both xUnit v2 packages
(xunit, xunit.extensibility.core) and xUnit v3 packages (xunit.v3,
xunit.v3.core, xunit.v3.extensibility.core, xunit.v3.runner.msbuild), which can
allow a consuming project to accidentally mix versions; audit all consuming test
.csproj files and ensure each explicitly references only one stack (either the
v2 package IDs or the v3 package IDs) by removing or overriding the other set
from their PackageReference entries (or add an MSBuild condition/props that
prevents simultaneous inclusion), and update team guidance or CI validation to
fail if a project contains both xunit and xunit.v3* references.
In `@editor-config/editorconfig-hierarchy/app-repository/src/NewInterface.cs`:
- Line 3: Rename the interface NewInterface to follow the configured naming rule
by prefixing it with 'I' (e.g., INewInterface); update the declaration of the
interface (NewInterface) and any references/usages throughout the codebase so
all references point to the new name to avoid compilation errors and satisfy the
naming convention.
In
`@editor-config/editorconfig-hierarchy/library-repository/using-shared-root-file.editorconfig`:
- Line 80: The EditorConfig entry for csharp_style_expression_bodied_operators
uses an invalid value "true:when_on_single_line"; update it to a valid
"value:severity" form (e.g., change to "true:suggestion" or another allowed
severity from silent|suggestion|warning|error) so tooling will recognize it, and
apply the same fix to the other five .editorconfig files that contain the same
invalid token.
In `@editor-config/StyleCopAnalyzersDefault.editorconfig`:
- Around line 127-130: Add the missing SA1502 rule by inserting the editorconfig
key dotnet_diagnostic.sa1502.severity = none between the existing SA1501 and
SA1503 entries so this file matches the other editorconfig files; specifically,
add a line setting dotnet_diagnostic.sa1502.severity = none immediately after
dotnet_diagnostic.SA1501.severity and before dotnet_diagnostic.SA1503.severity
to restore consistency.
In `@repository-config/resharper/file-layout-current-visual-studio-resharper.xml`:
- Around line 22-27: The XML rule under the <HasMember> block incorrectly uses
<And> to require both <HasAttribute Name="Xunit.FactAttribute" /> and
<HasAttribute Name="Xunit.TheoryAttribute" /> for methods (inside the <Kind
Is="Method" /> node); change the <And> to <Or> so the rule matches methods that
have either attribute. Also apply the same replacement for the similar
occurrence referenced at the other location (the block around the <HasAttribute
... Name="Xunit.FactAttribute" /> and <HasAttribute ...
Name="Xunit.TheoryAttribute" /> pair near lines 66-67) so both rule sets use
<Or> instead of <And>.
- Around line 83-88: The XML rule blocks that require a method to have all three
NUnit attributes are too strict; replace the surrounding <And> element with <Or>
for the blocks that contain <Kind Is="Method" /> together with <HasAttribute
Name="NUnit.Framework.TestAttribute" />, <HasAttribute
Name="NUnit.Framework.TestCaseAttribute" />, and <HasAttribute
Name="NUnit.Framework.TestCaseSourceAttribute" /> so the matcher accepts any one
of those attributes; make this change in both occurrences (the block containing
Kind Is="Method" and the three HasAttribute entries, and the second identical
block further down).
In `@repository-config/sample-solutions/libs-and-console-app/README.md`:
- Around line 8-12: The README in
repository-config/sample-solutions/libs-and-console-app uses incorrect relative
links; update the two links that currently point to ./LICENSE and
./repository-config/ so they resolve from this nested folder (replace them with
repository-root relative paths such as /LICENSE and /repository-config/ or
correct relative paths like ../../../LICENSE and ../../ to match your
preference), editing the README.md entries that reference LICENSE and the
repository-config folder so the links point to the actual targets.
---
Nitpick comments:
In `@development-notes-guidelines/coding-guidelines-notes.md`:
- Line 3: Replace the nonstandard header "CSharpCodingGuidelines Site" with the
conventional C# naming by updating that section heading to "C# Coding
Guidelines" (or "C# Coding Guidelines Site" if the word "Site" must be
preserved) so the header uses the "C#" token; locate the header text
"CSharpCodingGuidelines Site" and change it accordingly.
In `@development-notes-guidelines/findings.md`:
- Line 5: Replace the casual phrase "stuff I found" in the document
title/sentence with a clearer, professional phrase such as "findings and
observations", "notes and findings", or "identified findings" to improve
searchability and tone; update the sentence "This document contains stuff I
found and might want to review later." to a more precise variant like "This
document contains findings and observations to review later." and ensure similar
phrasing is used consistently throughout the file.
In `@development-notes-guidelines/net-analyzers.md`:
- Around line 9-20: Replace the raw URLs in the list with labeled Markdown links
using the package or repo names as the link text (e.g.,
"Philips.CodeAnalysis.MoqAnalyzers", "ErrorProne.NET.CoreAnalyzers",
"IDisposableAnalyzers", "Agoda.Analyzers",
"SmartAnalyzers.CSharpExtensions.Annotations",
"Philips.CodeAnalysis.MaintainabilityAnalyzers", "ReflectionAnalyzers",
"StructuredLogging.Analyzers", "Spectre.Console.Analyzer", "SerilogAnalyzer",
"TestableIO.System.IO.Abstractions.Analyzers", and "Gu.Analyzers") so each list
item reads like "PackageName — [PackageName](URL)"; keep the same URLs but use
the package/repo name as the label for readability and easier maintenance.
In `@scripts/add_context_menu.reg`:
- Around line 6-7: The registry script hardcodes a machine-specific Icon path
("Icon"="C:\\DevNet\\github\\open-with-claude-code\\claude.ico,0"); update the
"Icon" entries referencing claude.ico to use a relocatable approach: remove the
absolute C:\\DevNet path and either (a) use an environment-variable-based path
such as "%ProgramFiles%\\open-with-claude-code\\claude.ico,0" and store the
value as an expanded string (REG_EXPAND_SZ / hex(2)) in the .reg, or (b) omit
the Icon entries and document that installers must place claude.ico into a known
install folder and set the Icon at install time; change both "Icon" occurrences
accordingly so the .reg no longer contains a local-dev absolute path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b1e16828-79f6-4409-a6c9-b0280c9c5a3d
📒 Files selected for processing (21)
.gitignore.junie/guidelines.md.syncignoredependencies/Analyzers.Global.Packages.propsdependencies/Common.Packages.propsdependencies/MicrosoftExtensions.Net9.Packages.propsdependencies/Ploch.Packages.propsdependencies/Testing.Packages.propsdevelopment-notes-guidelines/coding-guidelines-notes.mddevelopment-notes-guidelines/findings.mddevelopment-notes-guidelines/net-analyzers.mdeditor-config/StyleCopAnalyzersDefault.editorconfigeditor-config/editorconfig-hierarchy/app-repository/.editorconfigeditor-config/editorconfig-hierarchy/app-repository/ClassLibrary1.csprojeditor-config/editorconfig-hierarchy/app-repository/src/NewClass.cseditor-config/editorconfig-hierarchy/app-repository/src/NewInterface.cseditor-config/editorconfig-hierarchy/library-repository/using-shared-root-file.editorconfigrepository-config/resharper/file-layout-current-visual-studio-resharper.xmlrepository-config/root-folder/.gitignorerepository-config/sample-solutions/libs-and-console-app/README.mdscripts/add_context_menu.reg
💤 Files with no reviewable changes (1)
- .junie/guidelines.md
| <PlochPackagesVersion>3.0.0</PlochPackagesVersion> | ||
| </PropertyGroup> | ||
| <ItemGroup> | ||
| <PackageVersion Include="Ploch.Data.GenericRepository" Version="2.1.0" /> | ||
| <PackageVersion Include="Ploch.Data.GenericRepository.EFCore" Version="2.1.0" /> | ||
| <PackageVersion Include="Ploch.Data.Model" Version="2.1.0" /> | ||
| <PackageVersion Include="Ploch.Common.AppServices" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.Data.GenericRepository" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.Data.GenericRepository.EFCore" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.Data.Model" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.Data.EFCore" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.Data.EFCore.IntegrationTesting" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.Data.EFCore.SqLite" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.Data.EFCore.SqlServer" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.Data.GenericRepository.EFCore.IntegrationTesting" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.Common" Version="$(PlochPackagesVersion)" /> | ||
| <PackageVersion Include="Ploch.Common.DependencyInjection" Version="$(PlochPackagesVersion)" /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all explicit Ploch package version declarations to detect local shadowing.
rg -nP --iglob '*.props' '<PackageVersion\s+Include="Ploch\.[^"]+"\s+Version="[^"]+"'Repository: mrploch/mrploch-development
Length of output: 2389
🏁 Script executed:
cat -n dependencies/Ploch.Packages.props | head -25Repository: mrploch/mrploch-development
Length of output: 1978
🏁 Script executed:
cat -n repository-config/sample-solutions/libs-and-console-app/directory.packages.propsRepository: mrploch/mrploch-development
Length of output: 2277
🏁 Script executed:
# Check if there are other explicit Ploch version overrides in the sample solution
rg -nP 'Version=' repository-config/sample-solutions/libs-and-console-app/directory.packages.propsRepository: mrploch/mrploch-development
Length of output: 1436
Centralized Ploch versioning is currently shadowed by a local override.
dependencies/Ploch.Packages.props (line 15) centralizes Ploch.Common to $(PlochPackagesVersion) (3.0.0), but repository-config/sample-solutions/libs-and-console-app/directory.packages.props (line 14) explicitly pins Ploch.Common to 2.0.1. This local override will shadow the centralized version when the props files are imported, creating version drift and weakening the centralization objective.
If 2.0.1 is not intentionally required for the sample solution, align it to 3.0.0:
Suggested fix
- <PackageVersion Include="Ploch.Common" Version="2.0.1" />
+ <PackageVersion Include="Ploch.Common" Version="3.0.0" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dependencies/Ploch.Packages.props` around lines 3 - 16, The sample-solution
local packages file is overriding the centralized PlochPackagesVersion by
pinning Ploch.Common to 2.0.1; update the sample-solution's
directory.packages.props so it no longer pins a conflicting version—either
remove the explicit Ploch.Common entry or change it to reference the centralized
variable/3.0.0 (i.e., align it with PlochPackagesVersion used in
Ploch.Packages.props) to ensure the central version (PlochPackagesVersion /
Ploch.Common) is honored across the repo.
There was a problem hiding this comment.
Acknowledged — tracked by #5. Will be split into separate PlochCommonPackagesVersion and PlochDataPackagesVersion in a follow-up PR.
There was a problem hiding this comment.
@kploch, understood! Deferring to a follow-up PR tracked by #5 makes sense. Looking forward to seeing PlochCommonPackagesVersion and PlochDataPackagesVersion introduced there to properly separate the versioning concerns.
✏️ Learnings added
Learnt from: kploch
Repo: mrploch/mrploch-development PR: 7
File: dependencies/Ploch.Packages.props:3-16
Timestamp: 2026-03-29T03:19:45.270Z
Learning: In `dependencies/Ploch.Packages.props` (mrploch/mrploch-development repo), the single `PlochPackagesVersion` property (currently `3.0.0`) is planned to be split into separate `PlochCommonPackagesVersion` and `PlochDataPackagesVersion` properties in a follow-up PR tracked by issue `#5`. Do not flag the unified `PlochPackagesVersion` as a long-term design decision; the split is intentionally deferred.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| @@ -0,0 +1,4 @@ | |||
| namespace Ploch.EditorConfig.FormattingCheck | |||
| { | |||
| public interface NewInterface { } | |||
There was a problem hiding this comment.
Interface naming conflicts with configured naming rule.
Line 3 uses NewInterface instead of INewInterface, which will violate the configured interface naming convention.
🔧 Proposed fix
- public interface NewInterface { }
+ public interface INewInterface { }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public interface NewInterface { } | |
| public interface INewInterface { } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@editor-config/editorconfig-hierarchy/app-repository/src/NewInterface.cs` at
line 3, Rename the interface NewInterface to follow the configured naming rule
by prefixing it with 'I' (e.g., INewInterface); update the declaration of the
interface (NewInterface) and any references/usages throughout the codebase so
all references point to the new name to avoid compilation errors and satisfy the
naming convention.
There was a problem hiding this comment.
False positive — sample file for editorconfig hierarchy testing, not production code.
There was a problem hiding this comment.
@kploch, understood — thanks for the clarification! Since these are intentional sample/placeholder files used to exercise the editorconfig hierarchy behavior, naming convention rules don't apply here.
( ˘▿˘ )~ ✏️
✏️ Learnings added
Learnt from: kploch
Repo: mrploch/mrploch-development PR: 7
File: editor-config/editorconfig-hierarchy/app-repository/src/NewInterface.cs:3-3
Timestamp: 2026-03-29T03:19:45.035Z
Learning: Files under `editor-config/editorconfig-hierarchy/` in the `mrploch/mrploch-development` repository are sample/placeholder files used for testing EditorConfig hierarchy behavior, not production code. Do not flag naming convention violations or other code style issues in these files.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| dotnet_diagnostic.SA1500.severity = warning # Braces for multi-line statements should not share line | ||
| dotnet_diagnostic.SA1501.severity = warning # Statement should not be on a single line | ||
| dotnet_diagnostic.SA1502.severity = warning # Element should not be on a single line | ||
|
|
||
| dotnet_diagnostic.SA1503.severity = warning # Braces should not be omitted |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify SA1502 severity declarations across editorconfig files.
rg -nP --iglob '*.editorconfig' 'dotnet_diagnostic\.(SA1502|sa1502)\.severity\s*='Repository: mrploch/mrploch-development
Length of output: 3583
🏁 Script executed:
# Check if the file exists and read the relevant section
if [ -f "editor-config/StyleCopAnalyzersDefault.editorconfig" ]; then
echo "=== Lines around 127-130 ==="
sed -n '125,135p' "editor-config/StyleCopAnalyzersDefault.editorconfig" | cat -n
echo ""
echo "=== Checking for SA1502 anywhere in file ==="
grep -in 'sa1502' "editor-config/StyleCopAnalyzersDefault.editorconfig" | head -20
else
echo "File not found: editor-config/StyleCopAnalyzersDefault.editorconfig"
fiRepository: mrploch/mrploch-development
Length of output: 1116
Add missing SA1502 rule to match other editorconfig files.
SA1502 is absent from this file, while every other editorconfig file in the repository explicitly sets dotnet_diagnostic.sa1502.severity = none. Add it between SA1501 and SA1503 for consistency.
Proposed fix
dotnet_diagnostic.SA1500.severity = warning # Braces for multi-line statements should not share line
dotnet_diagnostic.SA1501.severity = warning # Statement should not be on a single line
+dotnet_diagnostic.SA1502.severity = none # Element should not be on a single line
dotnet_diagnostic.SA1503.severity = warning # Braces should not be omitted📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| dotnet_diagnostic.SA1500.severity = warning # Braces for multi-line statements should not share line | |
| dotnet_diagnostic.SA1501.severity = warning # Statement should not be on a single line | |
| dotnet_diagnostic.SA1502.severity = warning # Element should not be on a single line | |
| dotnet_diagnostic.SA1503.severity = warning # Braces should not be omitted | |
| dotnet_diagnostic.SA1500.severity = warning # Braces for multi-line statements should not share line | |
| dotnet_diagnostic.SA1501.severity = warning # Statement should not be on a single line | |
| dotnet_diagnostic.SA1502.severity = none # Element should not be on a single line | |
| dotnet_diagnostic.SA1503.severity = warning # Braces should not be omitted |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@editor-config/StyleCopAnalyzersDefault.editorconfig` around lines 127 - 130,
Add the missing SA1502 rule by inserting the editorconfig key
dotnet_diagnostic.sa1502.severity = none between the existing SA1501 and SA1503
entries so this file matches the other editorconfig files; specifically, add a
line setting dotnet_diagnostic.sa1502.severity = none immediately after
dotnet_diagnostic.SA1501.severity and before dotnet_diagnostic.SA1503.severity
to restore consistency.
There was a problem hiding this comment.
This is a reference file documenting default StyleCop severity levels. The SA1502 setting here reflects the StyleCop default configuration, not our project-specific overrides. No action needed.
There was a problem hiding this comment.
@kploch, understood — the file documents StyleCop's own default severity levels, so omitting SA1502 is intentional (reflecting that StyleCop does not enforce it by default). I'll keep this in mind for future reviews.
✏️ Learnings added
Learnt from: kploch
Repo: mrploch/mrploch-development PR: 7
File: editor-config/StyleCopAnalyzersDefault.editorconfig:127-130
Timestamp: 2026-03-29T03:20:24.200Z
Learning: In the `mrploch/mrploch-development` repository, `editor-config/StyleCopAnalyzersDefault.editorconfig` is a reference file documenting StyleCop's default severity levels, not a project-specific override. Missing rules (e.g., SA1502) reflect StyleCop's own defaults, not an omission. Do not flag missing rules in this file for consistency with other editorconfig files.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: kploch
Repo: mrploch/mrploch-development PR: 7
File: editor-config/editorconfig-hierarchy/app-repository/src/NewInterface.cs:3-3
Timestamp: 2026-03-29T03:19:45.035Z
Learning: Files under `editor-config/editorconfig-hierarchy/` in the `mrploch/mrploch-development` repository are sample/placeholder files used for testing EditorConfig hierarchy behavior, not production code. Do not flag naming convention violations or other code style issues in these files.
Changelist by BitoThis pull request implements the following key changes.
|
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #fb5138
Actionable Suggestions - 4
-
scripts/add_context_menu.reg - 2
- Hard-coded icon path · Line 6-6
- Hard-coded icon path for background menu · Line 15-15
-
dependencies/Common.Packages.props - 1
- Coverage instrumentation change · Line 14-14
-
editor-config/editorconfig-hierarchy/library-repository/using-shared-root-file.editorconfig - 1
- ReSharper property typos · Line 507-508
Additional Suggestions - 3
-
scripts/add_context_menu.reg - 2
-
PATH assumption for claude command · Line 10-10The command assumes 'claude' is available in the system PATH, but if Claude Code CLI isn't installed globally or PATH isn't configured, this will fail with 'claude is not recognized'. Consider using the full executable path.
-
PATH assumption for background claude command · Line 19-19Similar to the directory command, this assumes 'claude' is in PATH for the background context menu. If not, the command will fail.
-
-
editor-config/editorconfig-hierarchy/library-repository/using-shared-root-file.editorconfig - 1
-
Redundant config duplicates · Line 224-225The file has duplicate settings for dotnet_diagnostic.cs8763.severity and cs8770.severity. EditorConfig uses the last value, making these redundant; remove them for cleaner config.
Code suggestion
@@ -210,6 +210,4 @@ - - dotnet_diagnostic.cs8763.severity = error # A method marked [DoesNotReturn] should not return. - dotnet_diagnostic.cs8770.severity = error # Method lacks [DoesNotReturn] annotation to match implemented or overridden member. - - ## Properties: 'dotnet_diagnostic.faa' - dotnet_diagnostic.faa0002.severity = warning + + ## Properties: 'dotnet_diagnostic.faa' + dotnet_diagnostic.faa0002.severity = warning
-
Review Details
-
Files reviewed - 14 · Commit Range:
c9e3510..c9e3510- .syncignore
- dependencies/Analyzers.Global.Packages.props
- dependencies/Common.Packages.props
- dependencies/MicrosoftExtensions.Net9.Packages.props
- dependencies/Ploch.Packages.props
- dependencies/Testing.Packages.props
- editor-config/StyleCopAnalyzersDefault.editorconfig
- editor-config/editorconfig-hierarchy/app-repository/.editorconfig
- editor-config/editorconfig-hierarchy/app-repository/ClassLibrary1.csproj
- editor-config/editorconfig-hierarchy/app-repository/src/NewClass.cs
- editor-config/editorconfig-hierarchy/app-repository/src/NewInterface.cs
- editor-config/editorconfig-hierarchy/library-repository/using-shared-root-file.editorconfig
- repository-config/root-folder/.gitignore
- scripts/add_context_menu.reg
-
Files skipped - 7
- .gitignore - Reason: Filter setting
- .junie/guidelines.md - Reason: Filter setting
- development-notes-guidelines/coding-guidelines-notes.md - Reason: Filter setting
- development-notes-guidelines/findings.md - Reason: Filter setting
- development-notes-guidelines/net-analyzers.md - Reason: Filter setting
- repository-config/resharper/file-layout-current-visual-studio-resharper.xml - Reason: Filter setting
- repository-config/sample-solutions/libs-and-console-app/README.md - Reason: Filter setting
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- Csharp (Linter) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at kris@ploch.dev.
Documentation & Help
AI Code Review powered by
| ; Add "Open with Claude Code" to folder context menu | ||
| [HKEY_CLASSES_ROOT\Directory\shell\OpenWithClaudeCode] | ||
| @="Open with Claude Code" | ||
| "Icon"="C:\\DevNet\\github\\open-with-claude-code\\claude.ico,0" |
There was a problem hiding this comment.
The icon path is hard-coded to a specific absolute location that may not exist on all systems, potentially resulting in no icon displaying in the context menu.
Code Review Run #fb5138
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
There was a problem hiding this comment.
Addressed in commit dcd3e77. Added a comment noting the icon path needs local adjustment.
There was a problem hiding this comment.
The review comment highlights a hardcoded local icon path in the new file scripts/add_context_menu.reg. Adding a comment notes the need for adjustment, but the path remains machine-specific and may not fully resolve the portability issue.
| ; Add "Open with Claude Code" to folder background context menu | ||
| [HKEY_CLASSES_ROOT\Directory\Background\shell\OpenWithClaudeCode] | ||
| @="Open with Claude Code" | ||
| "Icon"="C:\\DevNet\\github\\open-with-claude-code\\claude.ico,0" |
There was a problem hiding this comment.
Same hard-coded icon path issue for the background context menu.
Code Review Run #fb5138
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
There was a problem hiding this comment.
This question isn’t related to the pull request. I can only help with questions about the PR’s code or comments.
| <PackageVersion Include="CaseExtensions" Version="1.1.0" /> | ||
| <PackageVersion Include="CommunityToolkit.Mvvm" Version="8.4.0" /> | ||
| <PackageVersion Include="coverlet.msbuild" Version="6.0.4" /> | ||
| <PackageVersion Include="coverlet.msbuild" Version="8.0.0" /> |
There was a problem hiding this comment.
Updating coverlet.msbuild from 6.0.4 to 8.0.0 changes the default instrumentation heuristics. In prior versions, assemblies with any missing source files were excluded from coverage; now only assemblies with zero source files are excluded. This may result in different coverage reports. If the old behavior is needed, configure ExcludeAssembliesWithoutSources=MissingAny.
Code Review Run #fb5138
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
There was a problem hiding this comment.
Acknowledged. The instrumentation heuristic change in coverlet 8.0.0 is acceptable — we want assemblies with partial source coverage included. If issues arise, ExcludeAssembliesWithoutSources=MissingAny can be configured per-repo.
There was a problem hiding this comment.
This question isn’t related to the pull request. I can only help with questions about the PR’s code or comments.
| resharper_align_multline_type_parameter_constrains = true | ||
| resharper_align_multline_type_parameter_list = true |
There was a problem hiding this comment.
The ReSharper property names on these lines appear to have typos that could prevent the settings from being recognized. It looks like 'multline' should be 'multiline' and 'constrains' should be 'constraints' based on standard ReSharper EditorConfig documentation.
Review Rule
Verify Vendor-Specific Configuration Property Names Before Correction for repo: mrploch/mrploch-development, language: Other. Bito will avoid suggestions that match this rule. You can manage review rules here.Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
There was a problem hiding this comment.
False positive. resharper_align_multline_type_parameter_constrains and resharper_align_multline_type_parameter_list are the actual ReSharper EditorConfig property names — verified across ploch-common and ploch-data .editorconfig files, both generated by ReSharper/Rider. ReSharper uses multline (not multiline) for some properties and constrains (not constraints) for this specific one.
There was a problem hiding this comment.
You're right—ReSharper uses 'multline' and 'constrains' in these property names, as shown in the file. The suggestion to change them to 'multiline' and 'constraints' is a false positive based on standard ReSharper EditorConfig conventions.
- ReSharper file layout: Change <And> to <Or> for xUnit Fact/Theory and NUnit Test/TestCase/TestCaseSource attribute matching so test classes and methods are detected when using either attribute (not requiring all) - EditorConfig: Fix duplicated _highlighting suffix in async_apostle_configure_await setting - EditorConfig: Fix invalid format for expression_bodied_operators (was true:when_on_single_line, now when_on_single_line:silent) - EditorConfig: Remove stale TODO comment (investigation was completed) - .syncignore: Replace hardcoded project-specific .idea paths with generic glob patterns - add_context_menu.reg: Fix cmd.exe quoting (remove extra outer quotes), fix %v to %V for background context menu, add note about icon paths - README: Fix broken relative links in sample-solutions subdirectory Refs: #5 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Code Review Agent Run #fee752Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
1 participant
User description
Summary
Batch maintenance update for the
mrploch-developmentshared configuration repository.PlochPackagesVersionproperty (3.0.0); added all commonly used Ploch packages.gitignoretemplate, updated sample solution README.syncignore, ReSharper file layout XML, Windows context menu registry script for Claude Code.junie/guidelines.mdDependency Version Changes
Ploch.Packages.props Expansion
Introduced
PlochPackagesVersionMSBuild property (set to3.0.0) and added:Ploch.Common,Ploch.Common.DependencyInjection,Ploch.Common.Net9,Ploch.Common.DataAnnotations,Ploch.Common.AppServicesPloch.Data.EFCore,Ploch.Data.EFCore.IntegrationTesting,Ploch.Data.EFCore.SqLite,Ploch.Data.EFCore.SqlServerPloch.Data.GenericRepository.EFCore.IntegrationTestingPloch.TestingSupport,Ploch.TestingSupport.Xunit,Ploch.TestingSupport.FluentAssertionsPartially addresses #5.
Test plan
ploch-common,ploch-data,ploch-lists) restore and build successfully with updated package versions.editorconfigchanges do not introduce new warnings in existing repos.gitignoretemplate correctly excludes AI tool config filesCloses #6
🤖 Generated with Claude Code
CodeAnt-AI Description
Update shared development settings and package versions across the repo
What Changed
Impact
✅ Fewer dependency update conflicts✅ More consistent code formatting in new projects✅ Fewer accidental commits of local tool files💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
Chores
Configuration
Documentation
Tools
Summary by Bito
This maintenance update performs batch updates to the shared configuration repository, modernizing dependency versions and enhancing development tooling. The changes centralize package versioning, update code formatting standards, and add configuration for modern development workflows including AI tool integration. Key updates include bumping Microsoft Extensions and testing framework versions, expanding Ploch package management, removing obsolete StyleCop rules, and adding comprehensive formatting configurations.
Detailed Changes