search

cocoabits / MASShortcut

Modern framework for managing global keyboard shortcuts compatible with Mac App Store. More details:
http://blog.shpakovski.com/2012/07/global-keyboard-shortcuts-in-cocoa.html
BSD 2-Clause "Simplified" License
1.52k stars 220 forks source link

Fixed errors when building for Sierra (10.12) as deployment target #98

Closed clemens-schulz closed 8 years ago

clemens-schulz commented 8 years ago

I just replaced all deprecated enum names.

zoul commented 8 years ago

Thank you! If you keep the deployment target set to 10.6, do those warnings go away? We don’t want to bump the deployment target to 10.12 – there are production apps depending on MASShortcut that need to support older macOS versions.

shpakovski commented 8 years ago

Hello Clemens,

We cannot have this code yet because of compatibility with older platforms. Even Travis is not happy…

However, you may use preprocessor directives for conditional compilation, so that new constants would be used only when you build for 10.12. And for earlier SDKs, current code may remain and wait for deletion down the road.

Thanks in advance!

zoul commented 8 years ago

By the way: if the main motivation is building MASShortcut for Sierra, there should be no need to change the deployment target and the framework should build & run just fine. (If not, file an issue.)

clemens-schulz commented 8 years ago

Thanks, makes sense. I think silencing the deprecation warnings for this would be better than conditional compilation, because just the names changed and not the actual value of the constants.

How long do you plan to keep the base SDK <10.12? It wouldn't change the deployment target and it would allow support for the new touch bar (e.g. cancel button when setting a shortcut)

clemens-schulz commented 8 years ago

@zoul It builds without problems, but I'm using the framework in a project with 10.12 as deployment target. It works, but shows a few deprecation warnings in the framework header files.

zoul commented 8 years ago

I have filed an issue to silence (or otherwise solve) the deprecation warnings, will that do? (BTW, the base SDK is already set to latest, I just checked.)

clemens-schulz commented 8 years ago

Yes, the base SDK is always the current macOS version. But it looks like Travis is using an older version. My PR doesn't affect the deployment target, just the minimum base SDK.

zoul commented 8 years ago

I think we can’t accept the changes as suggested, since the project would no longer build on older systems, especially 10.11 (see the Travis failure log). But I can confirm that building a project with deployment target set to 10.12 results in the mentioned warnings. I am closing the PR, please let’s move to #99 to figure out how to silence the warnings in a backward-compatible way.