Cleaning Up

[paulrehkugler]

Resolved iOS7 deprecations and 64-bit warnings. Removing old changes I made that make no sense with the new iOS7 APIs. Converting some syntax to its "Modern Objective-C" equivalent.

[josharian]

This all looks very reasonable. Would you mind rebasing on top of josharian/master, though, to avoid churn? Some of these fixes are already in that branch.

Also, a general question -- there's some variation between using NS[U]Integer and [unsigned] [int|long], and I didn't see an obvious reason why that should be. All else being equal, I think we should either use NS[U]Integer or fixed-width integers (like uint8_t).

[paulrehkugler]

I'll give this a shot - actually using more than basic git functions requires a bit of Googling on my part.

According to StackOverflow, NSInteger/NSUInteger are "dynamic typedefs"... Will give different typedefs for different architecture: http://stackoverflow.com/a/4445199/953105. All I know is that on twitter everyone said "use NSInteger for 64-bit compatibility."

[josharian]

I'll give this a shot - actually using more than basic git functions requires a bit of Googling on my part.

No worries. Cherry-picking might be easier than rebasing in this case anyway. Just lmk when it is ready for me to take another look.

...NSInteger/NSUInteger...

Yep. The key goal here is just to use the right type in the right place. :)

[paulrehkugler]

Ok, everything's rebased on top of your master branch. Let me know if anything doesn't look right.

[josharian]

Why is TLLoadingView back? It got removed in 5773f638e29ef0e3c893958d3ca46e18cca6ebfe.

[josharian]

It looks like this work builds not on current master, but on your own master from some time ago, which is causing several fixes that occurred here to get undone by your changes.

[josharian]

I'm sorry to be a PITA, but in order to merge this with confidence, you should really make these changes starting from current master. (That's what I meant when I asked for a rebase.) If my request is still unclear, I apologize -- just let me know and I'll try to write it out in more detail.

[paulrehkugler]

Ok, I thought I did a rebase on top of your current master; maybe I did it wrong and you can help. Here are the steps I took to rebase:

git remote add dev git://github.com/josharian/xkcd.git
git fetch dev
git checkout dev
git rebase dev/master

Then I manually resolved merge conflicts (which accidentally added comicList.tableView deselectRowAtIndexPath:) and committed.

[josharian]

FWIW, the conventional name for the original remote is upstream, not dev.

That looks like roughly the right set of steps; I think the problem is that your master branch had diverged so far from mine that it was hard to resolve the conflicts correctly.

There are two alternatives to actually using rebase -- (1) merge upstream/master into origin/master, or (2) just use reset --hard origin/master to be upstream/master (danger of data loss!), and then cherry-pick in the commits you want. (The real goal was not necessarily to use rebase but to have the pull request be a series of small commits that applied cleanly and straightforwardly on top of master...because as you saw, merging wildly divergent branches provides too much opportunity for mistakes during merging.)

I took a stab at doing the latter for you:

https://github.com/josharian/xkcd/compare/master...paul

Please take a careful look through and let me know if that diff looks right to you.

At this point, the goal should be to (a) get these changes in, (b) get your master to be at the same commit as mine, and keep it there as much as possible, and (c) do future work in feature branches. That way we won't have these problems in the future.

[paulrehkugler]

Ok, thanks for that. I apologize that the Github for Mac app has me spoiled into not knowing much about git. I'll look over your cherrypick and let you know what I think.

Once I realized this would be a mess, I made feature branches for my other work here, so hopefully this will no longer be an issue in the future.

[paulrehkugler]

Yes - those changes look like what I did. I don't see anything strange added or anything missing. How do you want to continue? Close this PR and merge your branch without a PR, or close this and have me clone your branch and open a new PR? Something else?

[josharian]

I apologize that the Github for Mac app has me spoiled into not knowing much about git.

No worries. Learning git can be a pain, but it is time well spent, I assure you. FWIW, I use the command line and rowanj's excellent gitx fork. It's a bit closer to the metal, but it is worth it.

[josharian]

I've merged the paul branch I created into master (and you're still listed as the author on those commits), so I'm going to go ahead and just close this PR.

So the next and most important order of business for you is to make your fork's master exactly match my repo's master, and get your branches into good shape based on that common base.

The (dangerous! destructive!) way to do this is:

git fetch upstream
git checkout master
git checkout -b old_master # so that you can find your old master branch as needed
git checkout master # make master active again
git reset --hard upstream/master # this will completely reset your master branch to mine
# for each feature branch:
git checkout <some feature branch>
git rebase master

Make sure that you understand what each of those commands will do before executing them. :)

[paulrehkugler]

Thanks!

[paulrehkugler]

Also I think one of those lines is supposed to be git reset --hard upstream/master. I've picked up enough from the git masters to know that doesn't sound right.