[vlc-devel] [PATCH 1/1] growl: Add OS X user notifications as fallback to Growl notifications

Felix Paul Kühne fkuehne at videolan.org
Fri Oct 16 21:40:07 CEST 2015


Hello Marvin,

Looks good to me!

Minor nit-picks inline.

> On 16 Oct 2015, at 20:00, epirat07 at gmail.com wrote:
> 
> #import <Foundation/Foundation.h>
> +#import <AppKit/AppKit.h>

#import <Cocoa/Cocoa.h>

> @@ -72,11 +73,17 @@
>     NSString *o_applicationName;
>     NSString *o_notificationType;
>     NSMutableDictionary *o_registrationDictionary;
> +    id lastNotification;
> +    BOOL isInForeground;
> +    intf_thread_t *interfaceThread;

Inconsistent coding style. Would you mind updating the old variable names?

> 
> +            } else if (artistStr) {
> +                desc = [NSString stringWithFormat:@"%@", artistStr];

Save a memcpy and a selector dispatch here by directly assigning the pointer without creating a new string with a format operation.

> +            // Private APIs to set cover image and show action button
> +            [notification setValue:coverImage forKey:@"_identityImage"];
> +            [notification setValue:@(YES) forKey:@"_showsButtons”];

Can you please file a radar here and document it?

Best regards,

Felix


More information about the vlc-devel mailing list