Skip to content

Add macios-reviewer skill for PR code review#25206

Merged
rolfbjarne merged 5 commits intomainfrom
dev/rolf/macios-reviewer
Apr 23, 2026
Merged

Add macios-reviewer skill for PR code review#25206
rolfbjarne merged 5 commits intomainfrom
dev/rolf/macios-reviewer

Conversation

@rolfbjarne
Copy link
Copy Markdown
Member

Create a Copilot skill that reviews PRs against conventions distilled from
past reviews by senior maintainers (Sebastien, Rolf, Chris, Manuel, Alex).

Covers:

  • Binding definitions (Export selectors, NullAllowed, platform attributes,
    naming conventions, protocol patterns, breaking change guards)
  • MSBuild tasks and targets (Required properties, incremental builds,
    SessionId for remote execution)
  • Nullable reference types (no null-forgiving operator, ThrowIfNull)
  • Formatting (Mono style with tabs, space before parens)
  • Memory management (GC.KeepAlive, Dispose patterns, SIMD marshalling)
  • Performance, security, testing, and AI-specific pitfalls

Modelled after dotnet/android's reviewer skill with rules adapted
for this repository's Apple platform binding patterns.

Create a Copilot skill that reviews PRs against conventions distilled from
past reviews by senior maintainers (Sebastien, Rolf, Chris, Manuel, Alex).

Covers:
- Binding definitions (Export selectors, NullAllowed, platform attributes,
  naming conventions, protocol patterns, breaking change guards)
- MSBuild tasks and targets (Required properties, incremental builds,
  SessionId for remote execution)
- Nullable reference types (no null-forgiving operator, ThrowIfNull)
- Formatting (Mono style with tabs, space before parens)
- Memory management (GC.KeepAlive, Dispose patterns, SIMD marshalling)
- Performance, security, testing, and AI-specific pitfalls

Modelled after dotnet/android's reviewer skill with rules adapted
for this repository's Apple platform binding patterns.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new macios-reviewer Copilot skill intended to review dotnet/macios pull requests against repository-specific conventions (bindings, MSBuild, nullable, formatting, interop, etc.), with the rules captured as reference documentation alongside the skill definition.

Changes:

  • Introduce macios-reviewer skill definition and review workflow (SKILL.md).
  • Add a comprehensive rule/reference document to guide reviews (references/review-rules.md).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
.github/skills/macios-reviewer/references/review-rules.md Adds the detailed review rules/checklist intended to encode dotnet/macios conventions for automated PR review.
.github/skills/macios-reviewer/SKILL.md Defines the skill metadata and prescribes the workflow/comment format for running reviews using the rules.

Comment thread .github/skills/macios-reviewer/references/review-rules.md Outdated
Comment thread .github/skills/macios-reviewer/references/review-rules.md Outdated
Comment thread .github/skills/macios-reviewer/references/review-rules.md Outdated
Comment thread .github/skills/macios-reviewer/references/review-rules.md Outdated
Comment thread .github/skills/macios-reviewer/references/review-rules.md
Comment thread .github/skills/macios-reviewer/references/review-rules.md Outdated
rolfbjarne and others added 2 commits April 21, 2026 16:57
- Fix relative link to copilot-instructions.md
- Fix wrong selector error description (ObjC exception, not MissingMethodException)
- Add nullable parameters as trigger for custom delegates
- Fix TryCreate pattern description (returns bool + out, not status code)
- Replace C# formatting section: C# is auto-formatted, don't review it
- Update YAGNI section to reflect auto-formatting

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds a GitHub Agentic Workflow (.md) that triggers the macios-reviewer
skill when a maintainer comments '/review' on a pull request.

Uses claude-sonnet-4.5, reads the skill's review rules and methodology,
then posts inline review comments and a summary.

The .lock.yml will be auto-generated by the gh-aw compiler on first run.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@rolfbjarne rolfbjarne marked this pull request as ready for review April 21, 2026 15:22
@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

Copy link
Copy Markdown
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have to merge it to test it, but I would say go ahead and then comment /review on another PR.

I had to send a couple follow-ups after the first PR:

But you already have these in here ^^

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

Comment thread .github/workflows/macios-reviewer.md
@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

✅ [CI Build #180e507] Build passed (Build packages) ✅

Pipeline on Agent
Hash: 180e507f1373611d7508f01e4f82b52dc97e9355 [PR build]

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

✅ [PR Build #180e507] Build passed (Detect API changes) ✅

Pipeline on Agent
Hash: 180e507f1373611d7508f01e4f82b52dc97e9355 [PR build]

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

✅ API diff for current PR / commit

NET (empty diffs)

✅ API diff vs stable

NET (empty diffs)

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: 180e507f1373611d7508f01e4f82b52dc97e9355 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

✅ [CI Build #180e507] Build passed (Build macOS tests) ✅

Pipeline on Agent
Hash: 180e507f1373611d7508f01e4f82b52dc97e9355 [PR build]

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

🚀 [CI Build #180e507] Test results 🚀

Test results

✅ All tests passed on VSTS: test results.

🎉 All 156 tests passed 🎉

Tests counts

✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (iOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (MacCatalyst): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (macOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (Multiple platforms): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (tvOS): All 1 tests passed. Html Report (VSDrops) Download
✅ framework: All 2 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 4 tests passed. Html Report (VSDrops) Download
✅ generator: All 5 tests passed. Html Report (VSDrops) Download
✅ interdependent-binding-projects: All 4 tests passed. Html Report (VSDrops) Download
✅ introspection: All 6 tests passed. Html Report (VSDrops) Download
✅ linker: All 44 tests passed. Html Report (VSDrops) Download
✅ monotouch (iOS): All 11 tests passed. Html Report (VSDrops) Download
✅ monotouch (MacCatalyst): All 15 tests passed. Html Report (VSDrops) Download
✅ monotouch (macOS): All 12 tests passed. Html Report (VSDrops) Download
✅ monotouch (tvOS): All 11 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ sharpie: All 1 tests passed. Html Report (VSDrops) Download
✅ windows: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 4 tests passed. Html Report (VSDrops) Download
✅ xtro: All 1 tests passed. Html Report (VSDrops) Download

macOS tests

✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download
✅ Tests on macOS Ventura (13): All 5 tests passed. Html Report (VSDrops) Download
✅ Tests on macOS Sonoma (14): All 5 tests passed. Html Report (VSDrops) Download
✅ Tests on macOS Sequoia (15): All 5 tests passed. Html Report (VSDrops) Download
✅ Tests on macOS Tahoe (26): All 5 tests passed. Html Report (VSDrops) Download

Linux Build Verification

Linux build succeeded

Pipeline on Agent
Hash: 180e507f1373611d7508f01e4f82b52dc97e9355 [PR build]

Copy link
Copy Markdown
Member

@dalexsoto dalexsoto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@rolfbjarne rolfbjarne merged commit 15c3604 into main Apr 23, 2026
48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants