Skip to content
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

Xcode 7.3 updates #1353

Merged
merged 7 commits into from
Mar 23, 2016
Merged

Xcode 7.3 updates #1353

merged 7 commits into from
Mar 23, 2016

Conversation

ashfurrow
Copy link
Contributor

There are some Swift warnings, as discussed that I can look at when we're all moved to Xcode 7.3. When/if this is merged, I'll open an issue.

So, this PR moves Eigen to Xcode 7.3 and Swift 2.2. Everyone will need to use Xcode 7.3 after this is merged.

I generally like Xcode 7.3, especially the autocomplete. I will warn that Orta's snapshot plugin won't work because unit tests no longer print to the debugging console (going to try and take a look at this later, I think Orta already had a solution in mind).

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @orta to be a potential reviewer

@@ -189,7 +189,7 @@ - (void)addArtworksInShow:(PartnerShow *)show atPage:(NSInteger)page toView:(ARA
{
__weak typeof(self) wself = self;
[self getArtworksInShow:show atPage:page success:^(NSArray *artworks) {
if (!artworks.count > 0) { return; }
if (artworks.count < 1) { return; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, jeez. Can’t this be simplified to count == 0 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure can, it's a preference/style thing I guess. I usually go with < 1 Swift has an isEmpty that I normally use.

Copy link
Contributor

Choose a reason for hiding this comment

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

I like equal comparison, because with < I have to think for a split second whether or not this value could really be a negative value.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that checking for ‘empty’ is much nicer.

@ArtsyOpenSource
Copy link
Contributor

1 Warning
⚠️ Needs testing on a Phone if change is non-trivial

Generated by 🚫 danger

@alloy
Copy link
Contributor

alloy commented Mar 23, 2016

Looks good, hello future!

@@ -26,6 +26,7 @@ upcoming:
- Fixes cell bottom margin issues. - ash
- Fix hero unit description not fully showing when spanning multiple lines. - alloy
- Fix auction related analytics. - orta + alloy
- Updates for Xcode 7.3. - ash
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, this is not right, 2.4.1 has been submitted to the app store, so this needs to be in a new section.

@ashfurrow
Copy link
Contributor Author

🍏

@alloy
Copy link
Contributor

alloy commented Mar 23, 2016

Parfait 👍

@alloy alloy merged commit 89c281b into master Mar 23, 2016
@alloy alloy deleted the ashfurrow-xcode-7.3-updates branch March 23, 2016 20:42
@alloy
Copy link
Contributor

alloy commented Mar 23, 2016

@sarahscott @mennenia Be sure to update your Xcodes

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