search

TheAlmightyBob / Calendars

Cross-platform calendar API plugin for Xamarin and Windows
MIT License
101 stars 23 forks source link

Master #27

Closed bmacombe closed 8 years ago

bmacombe commented 8 years ago

Added NSCalendarsUsageDescription to info.plist to allow the test project to run on iOS 10

bmacombe commented 8 years ago

Maybe I got it to work correctly this time!

TheAlmightyBob commented 8 years ago

The checks passed, but we still have 7 commits here when we only need one. Indeed, one of those commits is even the merge from master. This is where rebasing would help, as it would re-apply your changes on top of the latest master changes instead of creating a new merge commit. (and an interactive rebase would let you delete/combine the excess commits)

It also looks like you've just been working in the master branch of your fork... I believe it is generally considered better to leave that alone and do your work in separate feature branches (would've made it easier to ditch your first attempt and start over, and I believe easier to rebase, among other things)

However, in this case it looks like GitHub itself offers a "squash" option for the pull request... so I'll give that a shot...

Actually, no I spoke too soon. Need to get rid of that extra Info.plist that it's trying to add back in.

bmacombe commented 8 years ago

I can give it another try...sorry this is such a pain for simple change...I'm fumbling my way through this. If you like you could just add the key to the plist your self and not worry about the pull request while I play with git more.

TheAlmightyBob commented 8 years ago

I prefer that contributors get proper credit in the history if possible. :)

But since this is a crash it should certainly get dealt w/ asap.

At this point, you could probably just add one more commit that removes Info.plist from Calendars.Plugin.iOS.Tests, and then I can merge&squash from there.

Also, it's helpful if you include a reference to the issue you're addressing (#24), so GitHub can link that up.