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

Added support for pulling the owners profile photo from the addressbook. #4

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

touchbee
Copy link

No description provided.

@jakeboxer
Copy link
Owner

Hey @touchbee,

This looks pretty good. A few quick changes that would be great before I merge this:

  1. Remove the logic for < iOS 4.1. This framework requires iOS 5+ to run anyways.
  2. Change the check for "ipod touch" to just "touch". The way it is now, if the device name is "First Last's iPod Touch", I believe it'll be changed to "First Last Touch".
  3. Try to stay consistent with my coding style. Your indentation and spacing looks a bit different than mine (which is fine normally, but I'd like to keep it consistent within this project).

@touchbee
Copy link
Author

Hi Jake,

  1. I'll check that out.
  2. Yeah I think I'm using the default indentation. Wondered why it looks like that. What indentation are you using? 2 spaces?

Removed image retrieval for iOS < 4.2.
Reformatted code.
@jakeboxer
Copy link
Owner

@touchbee yep, 2 spaces. Also, keep an eye on other little things. For example, I don't do spaces inside parentheses or a blank line after every line of code.

@horak
Copy link

horak commented Oct 15, 2013

Hi @touchbee @jakeboxer, I'd love to see this PR get merged in for a project I'm working on! Could you take another look?

@dmyers
Copy link

dmyers commented Mar 21, 2015

👍

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