Skip to content

Commit d679f2b

Browse files
[copilot-instructions] API design rules from PR #305 review (#317)
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: Jonathan Peppers <jonathan.peppers@microsoft.com>
1 parent 2bea0eb commit d679f2b

1 file changed

Lines changed: 3 additions & 0 deletions

File tree

.github/copilot-instructions.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ When setting environment variables for SDK tools (e.g. `sdkmanager`, `avdmanager
4848

4949
- **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.
5050
- **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.
51+
- **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.
52+
- **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.
53+
- **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.
5154
- **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.
5255
- **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.
5356
- **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`.

0 commit comments

Comments
 (0)