[skills] Add macios-binding-creator skill v1#24937
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
rolfbjarne
left a comment
rolfbjarne
left a comment
There was a problem hiding this comment.
Looks good! Just a few minor things.
I'm not sure if it should be a different skill, but having Copilot write unit tests for "manual" APIs would also be nice.
| // Custom result type — generates Task<FetchResult> FetchValuesAsync () | ||
| [Export ("fetchMultipleValues:")] | ||
| [Async (ResultTypeName = "FetchResult")] | ||
| void FetchValues (Action<string, nint, NSError> completion); |
There was a problem hiding this comment.
This should use a custom delegate instead of Action (so that we can specify nullability).
| [StructLayout (LayoutKind.Sequential)] | ||
| public struct MyStruct { | ||
| public nfloat X; | ||
| public nfloat Y; |
There was a problem hiding this comment.
I think we should prefer private fields + public properties, instead of public fields, because then it becomes easier to fix the struct layout later on if we made a mistake (or Apple changes things)
| ```csharp | ||
| // Methods that take NSError** | ||
| [Export ("doSomething:")] | ||
| bool DoSomething (out NSError error); |
There was a problem hiding this comment.
| bool DoSomething (out NSError error); | |
| bool DoSomething ([NullAllowed] out NSError error); |
| - **Selectors**: Must match exactly — a single typo causes runtime crashes. | ||
| - **Protocol conformance**: All `[Abstract]` methods in a protocol are required. | ||
| - **nint/nuint**: Use `nint`/`nuint` for Objective-C `NSInteger`/`NSUInteger`. | ||
|
|
There was a problem hiding this comment.
- Acronyms shouldn't be all uppercase (SIMD -> Simd, ID (when it means "identifier") -> Id, URL -> Url, etc.)
- Methods should be verbs. Properties should be nouns.
This comment has been minimized.
This comment has been minimized.
yeah this is just to bootstrap the skill it will definitely be updated as we find more things let's experiment more and figure out where to add it, I am leaning towards that be part of this workflow as well, once I run into it will do it. |
✅ [CI Build #0a90f42] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #0a90f42] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
✅ [CI Build #0a90f42] Build passed (Build macOS tests) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build #0a90f42] 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 macOS tests✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
3 participants
No description provided.