From 2129a6c1d09a606854844cf6abd7ff7daa318d3a Mon Sep 17 00:00:00 2001 From: Rui Marinho Date: Thu, 19 Mar 2026 17:32:32 +0000 Subject: [PATCH 1/2] Update copilot-instructions: strongly-typed APIs, no convenience overloads, stdout in errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add three rules learned from PR #305 review feedback: - Prefer strongly-typed APIs (enum+record) over string parameters - Avoid convenience overloads (string, int, typed) — pick one - Include stdout in ProcessUtils.ThrowIfFailed error diagnostics Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index a18916c4..e01d3c7a 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -48,6 +48,9 @@ When setting environment variables for SDK tools (e.g. `sdkmanager`, `avdmanager - **One type per file**: each public class, struct, enum, or interface must be in its own `.cs` file named after the type (e.g. `JdkVersionInfo` → `JdkVersionInfo.cs`). Do not combine multiple top-level types in a single file. - **Minimal public API**: prefer `internal` for new methods/classes unless they are consumed by external projects (dotnet/android, IDE extensions). Use `InternalsVisibleTo` for test access. +- **Strongly-typed APIs over strings**: when an API parameter has a finite set of valid forms (e.g. `"tcp:5000"`), use a record/enum pair (e.g. `AdbPortSpec(AdbProtocol.Tcp, 5000)`) instead of raw strings. Callers get compile-time safety, IntelliSense, and pattern matching. +- **Avoid convenience overloads**: don't add `string`, `int`, and strongly-typed overloads for the same method. Pick the strongly-typed signature and let callers construct the type. Fewer overloads = smaller API surface, fewer RS0026/RS0027 suppressions, and less maintenance. +- **Include stdout in error diagnostics**: when a method captures stdout (e.g. `ListReversePortsAsync`), pass it to `ProcessUtils.ThrowIfFailed(exitCode, command, stderr, stdout)` so failure messages include all output, not just stderr. - **Update PublicAPI files**: when adding or removing `public` API surface, update the `PublicAPI.Unshipped.txt` files under `src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/net10.0/` and `src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/netstandard2.0/`. New types and members go in the unshipped file. Build with `--no-incremental` and verify zero `RS0016` (missing) or `RS0017` (removed) warnings. See `PublicAPI.Shipped.txt` for the expected entry format. - **Use `ProcessUtils`**: never use `System.Diagnostics.Process` directly. Use the existing helpers such as `ProcessUtils.CreateProcessStartInfo()`, `ProcessUtils.StartProcess()`, and `ProcessUtils.ExecuteToolAsync()` for launching external tools. This ensures consistent logging, timeout handling, and cancellation. - **Process arguments**: use `ProcessUtils.CreateProcessStartInfo()` and pass arguments as separate strings instead of building a single arguments string yourself. `ProcessUtils` will use `ProcessStartInfo.ArgumentList` when available and fall back to `Arguments` on `netstandard2.0`. From 359dc6d462a320e77665999cfc5e52dfb5e4647e Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 19 Mar 2026 13:13:51 -0500 Subject: [PATCH 2/2] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index e01d3c7a..00483487 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -49,7 +49,7 @@ When setting environment variables for SDK tools (e.g. `sdkmanager`, `avdmanager - **One type per file**: each public class, struct, enum, or interface must be in its own `.cs` file named after the type (e.g. `JdkVersionInfo` → `JdkVersionInfo.cs`). Do not combine multiple top-level types in a single file. - **Minimal public API**: prefer `internal` for new methods/classes unless they are consumed by external projects (dotnet/android, IDE extensions). Use `InternalsVisibleTo` for test access. - **Strongly-typed APIs over strings**: when an API parameter has a finite set of valid forms (e.g. `"tcp:5000"`), use a record/enum pair (e.g. `AdbPortSpec(AdbProtocol.Tcp, 5000)`) instead of raw strings. Callers get compile-time safety, IntelliSense, and pattern matching. -- **Avoid convenience overloads**: don't add `string`, `int`, and strongly-typed overloads for the same method. Pick the strongly-typed signature and let callers construct the type. Fewer overloads = smaller API surface, fewer RS0026/RS0027 suppressions, and less maintenance. +- **Avoid convenience overloads**: don't add `string`, `int`, and strongly-typed overloads for the same method. Pick the strongly-typed signature and let callers construct the type. Fewer overloads = smaller API surface, fewer RS0027 suppressions, and less maintenance. - **Include stdout in error diagnostics**: when a method captures stdout (e.g. `ListReversePortsAsync`), pass it to `ProcessUtils.ThrowIfFailed(exitCode, command, stderr, stdout)` so failure messages include all output, not just stderr. - **Update PublicAPI files**: when adding or removing `public` API surface, update the `PublicAPI.Unshipped.txt` files under `src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/net10.0/` and `src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/netstandard2.0/`. New types and members go in the unshipped file. Build with `--no-incremental` and verify zero `RS0016` (missing) or `RS0017` (removed) warnings. See `PublicAPI.Shipped.txt` for the expected entry format. - **Use `ProcessUtils`**: never use `System.Diagnostics.Process` directly. Use the existing helpers such as `ProcessUtils.CreateProcessStartInfo()`, `ProcessUtils.StartProcess()`, and `ProcessUtils.ExecuteToolAsync()` for launching external tools. This ensures consistent logging, timeout handling, and cancellation.