-
Notifications
You must be signed in to change notification settings - Fork 125
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 INKMailHandler for Mail and Google Mail #16
Conversation
|
||
- (instancetype)init { | ||
if (self = [super init]) { | ||
self.subject = nil; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These initializations to nil aren't actually necessary. You can safely remove the custom init method entirely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, sure. Does the property always defaults to nil? On some other platforms a lot initialised variables are initialised to rubbish. BOOL
on iOS for example as well.
Edit: Yeah, ok. Properties defaults to nil.
How convenient. ;)
Great! Just left a few minor comments, then this should be good to go. |
Isn't the |
Cool. One last minor thing — "Google Mail" should probably be renamed to Gmail, since that's what the app itself is called. (I tried to do it myself, but it doesn't look like there's a good way for me to add to an in-progress PR for a fork I don't own) |
HaHa. Had it called GMail, but renamed it to Google Mail. :D |
Cool. Just to clarify, the official spelling is 'Gmail', with a lowercase 'm'. |
HA. That's difficult to change, with a non case-sensitive file system ... |
Sorry to be such a fussbudget, but the name inside the plist is still "Google Mail". Also, any chance you can squash these down to a single commit? |
You should be able to do the filename changes with case sensitivity pretty easily in the Finder. |
Squashed and passed test. Here you go. ;) |
added INKMailHandler for Mail and Google Mail
No description provided.