Skip to content

Update copilot-instructions: API design rules from PR #305 review #317

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.

Sign up for GitHub

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

Merged
jonathanpeppers merged 2 commits into from
Mar 19, 2026
Merged

Update copilot-instructions: API design rules from PR #305 review #317

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2129a6c
Update copilot-instructions: strongly-typed APIs, no convenience over…
rmarinho Mar 19, 2026
359dc6d
Potential fix for pull request finding
jonathanpeppers Mar 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/copilot-instructions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 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`.
Expand Down