{"action":"created","issue":{"url":"https://api.github.com/repos/philihp/SnoopTwitter/issues/1","repository_url":"https://api.github.com/repos/philihp/SnoopTwitter","labels_url":"https://api.github.com/repos/philihp/SnoopTwitter/issues/1/labels{/name}","comments_url":"https://api.github.com/repos/philihp/SnoopTwitter/issues/1/comments","events_url":"https://api.github.com/repos/philihp/SnoopTwitter/issues/1/events","html_url":"https://github.com/philihp/SnoopTwitter/issues/1","id":158532526,"number":1,"title":"[Intro to iOS] Project 3 - Review my app","user":{"login":"philihp","id":1247668,"avatar_url":"https://avatars.githubusercontent.com/u/1247668?v=3","gravatar_id":"","url":"https://api.github.com/users/philihp","html_url":"https://github.com/philihp","followers_url":"https://api.github.com/users/philihp/followers","following_url":"https://api.github.com/users/philihp/following{/other_user}","gists_url":"https://api.github.com/users/philihp/gists{/gist_id}","starred_url":"https://api.github.com/users/philihp/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/philihp/subscriptions","organizations_url":"https://api.github.com/users/philihp/orgs","repos_url":"https://api.github.com/users/philihp/repos","events_url":"https://api.github.com/users/philihp/events{/privacy}","received_events_url":"https://api.github.com/users/philihp/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2016-06-05T01:28:40Z","updated_at":"2016-06-06T21:48:06Z","closed_at":null,"body":"My app is \"complete\", please review. /cc @codepathreview"},"comment":{"url":"https://api.github.com/repos/philihp/SnoopTwitter/issues/comments/224098982","html_url":"https://github.com/philihp/SnoopTwitter/issues/1#issuecomment-224098982","issue_url":"https://api.github.com/repos/philihp/SnoopTwitter/issues/1","id":224098982,"user":{"login":"nickai","id":4792749,"avatar_url":"https://avatars.githubusercontent.com/u/4792749?v=3","gravatar_id":"","url":"https://api.github.com/users/nickai","html_url":"https://github.com/nickai","followers_url":"https://api.github.com/users/nickai/followers","following_url":"https://api.github.com/users/nickai/following{/other_user}","gists_url":"https://api.github.com/users/nickai/gists{/gist_id}","starred_url":"https://api.github.com/users/nickai/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nickai/subscriptions","organizations_url":"https://api.github.com/users/nickai/orgs","repos_url":"https://api.github.com/users/nickai/repos","events_url":"https://api.github.com/users/nickai/events{/privacy}","received_events_url":"https://api.github.com/users/nickai/received_events","type":"User","site_admin":false},"created_at":"2016-06-06T21:48:06Z","updated_at":"2016-06-06T21:48:06Z","body":"Nice work Philihp! We designed this homework so that you can get experience building an app that's more similar in complexity to a production app. It's also useful to have practical experience building a container view controller, which is a more advanced topic. The optional account switching feature is interesting because you have to think about how TwitterClient is caching the access token to make sure it works correctly for multiple accounts.\r\n\r\nA couple of notes after checking out your code / gif:\r\n\r\n* Nice to see you add in the new tweet to the list of tweets locally without needing to pull to refresh!\r\n* Make sure you are calling all the view controller lifecycle methods in your `ContainerViewController`. In particular, I don't see you calling [didMovetoParentViewController](https://developer.apple.com/library/ios/documentation/UIKit/Reference/UIViewController_Class/index.html#//apple_ref/occ/instm/UIViewController/didMoveToParentViewController:) anywhere.\r\n* I don't think you need the call to [remove the last item in the tweets array](https://github.com/philihp/SnoopTwitter/blob/bbf69606e80eabb948dbbf52a3205336b4c570a5/SnoopTwitter/TweetsViewController.swift#L90). When you call `reloadData` on the tableView, it will trigger a new call to `numberOfRowsInSection` which will return the current length of the `tweets` array.\r\n* For any cell configuration that will apply to all cells, you'll want to add this code to `awakeFromNib` (i.e. setting the profileImage [cornerRadius](https://github.com/philihp/SnoopTwitter/blob/master/SnoopTwitter/TweetCell.swift#L24)).\r\n\r\nYou can also find our overall Twitter assignment feedback guide below:\r\n\r\n* **Each .xib file or Storyboard scene should have Auto Layout constraints specified for each view.** There should be no yellow Auto Layout warnings, even if the layout seems to work. Auto Layout warnings are like having bad HTML. The browser will try to render it, and it might even look right, but it won't be fully predictable.\r\n* **Is your TweetsViewController generic enough that it can be reused to display tweets from other Twitter endpoints?** A view controller that displays a list of tweets is the type of view controller that you want to make generic enough that it can be used in multiple places throughout an app. This particular TweetsViewController can come in handy for displaying the user's home timeline, but can also be reused for displaying a list of tweets that mention the particular user, showing another user's timeline, etc.\r\n* **Did you properly handle the conditionally present label \"retweeted by...\" in your custom TweetCell?** You'll notice that for some tweets you'll need to display the \"retweeted by...\" label and have this hidden for other tweets. You'll want to create an outlet for a top margin AutoLayout constraint and programatically modify the constant depending on whether you want to show the label or not.\r\n* **Did you properly format the timestamp of each Tweet?** You'll notice that in the actual Twitter app the timestamps for each tweet are presented in a friendly format (i.e. \"10m\", \"1h\", etc). Since this isn't how the timestamp is returned from the server, you'll need to do the conversion yourself. [DateTools](https://github.com/MatthewYork/DateTools) is a handy CocoaPod that does this for you.\r\n* **Did you handle reply/retweet/favorite actions from within the TweetCell?** This is an example where it's useful for your model to be able to handle network actions, so you can simply wire up the buttons to a method like `tweet.retweet()`. You can even pass in an optional block if you want to handle a failure of the network event.\r\n* **In your ComposeViewController, did you pass back the Tweet after it was successfully created?** This will allow you to locally insert newly created tweets so you don't have to do a network refresh to see the new tweets.\r\n* **Did you immediately update the retweet and favorite counts when a user retweets or favorites a tweet?** It's common practice when building an app to have actions like favoriting a tweet assume success and ignore any failures. This gives the user an immediate response to his/her action without having to wait for a network response.\r\n* **Did you share code between your ProfileViewController and TimelineViewController?** The `ProfileViewController` and the `TimelineViewController` both contain a feed of tweets. There are three general strategies: you can have a single view controller that conditionally has the profile header, you can create the ProfileViewController as a subclass of the TimelineViewController, or you can create a separate class called a TimelineController (not a view controller). TimelineController is a dataSource and a delegate to a UITableView and knows how to show a feed of tweets.\r\n* **Did you design the HamburgerViewController so that it can be reused in other apps?** The HamburgerViewController should be designed to be agnostic to the Twitter app. In other words, have you designed it to be reusable in another application?\r\n* **Did you make sure to trigger the proper view controller lifecycle methods when adding or removing a child view controller to your container view controller?** Make sure to review the Apple guide on [Container View Controllers](https://developer.apple.com/library/ios/featuredarticles/ViewControllerPGforiPhoneOS/ImplementingaContainerViewController.html) to ensure you've triggered the proper lifecycle methods. These lifecycle methods should be triggered for ***both*** the `ContentViewController` and the `MenuViewController`.\r\n* **Have you implemented property observers for properties where appropriate?** When setting the `MenuViewController` and `ContentViewController` in the `HamburgerViewController`, it's a good idea to use property observers to call the appropriate view controller lifecycle methods.\r\n* **Did you properly format numbers like the favorite count?** In most apps you'll notice that large numbers will be formatted like \"10.8M\" instead of \"10,800,000\".\r\n"}}
{"actor":{"display_login":"nickai"}}