Skip to content

Conversation

@VincentDondain
Copy link
Contributor

Note: Beta 5 and Beta 6 changes canceled each others.

@monojenkins
Copy link
Collaborator

Build failure

@VincentDondain
Copy link
Contributor Author

@spouliot Introspection-mac work great locally.

src/modelio.cs Outdated
// Added in iOS 10 SDK but it is supposed to be present in iOS 9.
[Mac (10,12)]
[Export ("colorSpace")]
string ColorSpace { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

that's a weird choice for a type, i.e. it's not a CGColorSpace. Can you double check the online documentation and the header file to see if there's any detail on its usage ?

I suspect it might be constants, and NSString might be better in such case to avoid conversions, but I don't recall seeing field-based colourspace constants...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@spouliot there is 0 information about it, in neither the header nor the online doc ):

Do you want me to make it an NSString?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, it's unlikely to be a user string and, sometimes, constants do pointer comparison so we better (without any doc) not take a chance

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright fixed.

@spouliot
Copy link
Contributor

LGTM but please check my comment about the ColorSpace property

@monojenkins
Copy link
Collaborator

Build success

@spouliot spouliot merged commit ba196a8 into dotnet:xcode8 Aug 23, 2016
@VincentDondain VincentDondain deleted the modelio-b6 branch August 23, 2016 16:50
rolfbjarne added a commit to rolfbjarne/macios that referenced this pull request Feb 20, 2018
rolfbjarne added a commit to rolfbjarne/macios that referenced this pull request Feb 20, 2018
rolfbjarne added a commit that referenced this pull request Feb 21, 2018
rolfbjarne added a commit to rolfbjarne/macios that referenced this pull request Feb 21, 2018
rolfbjarne added a commit to rolfbjarne/macios that referenced this pull request Feb 21, 2018
rolfbjarne added a commit that referenced this pull request Feb 21, 2018
spouliot pushed a commit to spouliot/xamarin-macios that referenced this pull request Mar 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants