-
Notifications
You must be signed in to change notification settings - Fork 514
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.
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
Fix #9996 and #15622 #15642
Fix #9996 and #15622 #15642
Conversation
…of CPListSection
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we need to Obsolete and leave the old ones while providing a path for users to upgrade rather that break the API. Nevertheless I have triggered the build to confirm the API will be broken.
@janwiebe-jump Thanks for the fix! I have some small concerts which I left in the review. |
Added Items[] as well, to conform to iOS SDK https://developer.apple.com/documentation/carplay/cplistsection
Thanks @mandel-macaque , good catch indeed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still need to wrap the old api with a #if so that next time we have the chance to break APIs we do not expose something wrong :)
Co-authored-by: Manuel de la Pena <mandel@microsoft.com>
Co-authored-by: Manuel de la Pena <mandel@microsoft.com>
Co-authored-by: Manuel de la Pena <mandel@microsoft.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets see what the tests got to see. But the changes look good to me.
/azp run |
Hmm, the
Thanks. I am curious how the duplicate |
Azure Pipelines successfully started running 1 pipeline(s). |
@janwiebe-jump I suspect is going to fail, but it an easy fix (ugly, but doable). I'll keep an eye as according tot he failures I might propose a code change. |
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.
Co-authored-by: Manuel de la Pena <mandel@microsoft.com>
Co-authored-by: Alex Soto <alex@alexsoto.me>
Co-authored-by: Alex Soto <alex@alexsoto.me>
Co-authored-by: Alex Soto <alex@alexsoto.me>
/azp run |
This comment has been minimized.
This comment has been minimized.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Looks like our CI is having a hard time :( |
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.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
It doesn't compile:
|
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
🔥 Failed to compare API and create generator diff 🔥 Error: 'make' failed for the hash 1f12270. Pipeline on Agent |
💻 [PR Build] Tests on macOS Mac Catalina (10.15) passed 💻✅ All tests on macOS Mac Catalina (10.15) passed. Pipeline on Agent |
❌ [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) failed ❌Failed tests are:
Pipeline on Agent |
🚀 [CI Build] Test results 🚀Test results✅ All tests passed on VSTS: simulator tests. 🎉 All 223 tests passed 🎉 Tests counts✅ bcl: All 69 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
Thanks a lot for your help! |
Hi @rolfbjarne, |
@janwiebe-jump yes, this will be released in a xamarin-ios.pkg (when we release support for Xcode 14, the packages will contain your fix) |
As discussed in #9996 the CPListSection constructors are not fully correct, since the CPListImageRowItem can't be added to it, which is possible in the native iOS SDK.
This PR fixes that. With that, also #15622 seems to be fixed.