Skip to content
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

Reduce the number of compiler warnings #834

Merged
merged 11 commits into from Apr 26, 2012
Merged

Conversation

HenningJ
Copy link
Contributor

@HenningJ HenningJ commented Apr 22, 2012

I tried to reduce the number of warnings that crop up while compiling. This number has been raising steadily over time, so by now you wouldn't even notice new ones anymore. While QS still compiles and works even with all those warnings, it is a bad sign to have many warnings. Sometimes a warning may indicate a serious problem and if we don't notice new warnings, we wont catch the really bad ones.

Therefore, I reduced the number of warnings from somewhere around 40 to 14 (I think) on my Mac. In the future I'd like to amp up the compiler's warning sensitivity and treat warnings like errors and not merge anything that adds warnings. But first, reduce the number of warnings at the current warning level to 0 and keep it that way. So this is a step in the right direction.

Something about commit b191510: For some reason, my IB didn't recognize some of the outlets anymore, even though they were actually there. I think that might have happen when someone with XCode 4 open (and saved) the .xib files. I think @skurfer found some kind of compatibility setting that fixes that, right?

@HenningJ
Copy link
Contributor Author

@HenningJ HenningJ commented Apr 22, 2012

Just for fun, I compiled QS with the -Wextra warning flag turned on. This resulted in about 800 warnings. Granted, most of them were "unused parameter" warnings, which (most likely) don't matter. But still, interesting. :-)

@skurfer
Copy link
Member

@skurfer skurfer commented Apr 23, 2012

I was thinking about doing something like this recently. Nice to see you beat me to it.

I reduced the number of warnings from somewhere around 40 to 14 (I think) on my Mac.

For me, it goes from 93 down to 68. So I may get a chance to work on this after all. :-/

I think @skurfer found some kind of compatibility setting that fixes that, right?

You mean the auto-layout thing? That shouldn’t be an issue from simply opening and saving an existing NIB. (Unless the person explicitly enabled auto-layout in the process, but I don’t think any of us have ever done that. I don’t think it was even there prior to Xcode 4.3.) There must be some other incompatible new feature. What was the warning?

@HenningJ
Copy link
Contributor Author

@HenningJ HenningJ commented Apr 24, 2012

You mean the auto-layout thing? That shouldn’t be an issue from simply opening and saving an existing NIB. (Unless the person explicitly enabled auto-layout in the process, but I don’t think any of us have ever done that. I don’t think it was even there prior to Xcode 4.3.) There must be some other incompatible new feature.

I just remembered that there was something with the Remote Hosts Plugin pref pane and thought you might know more.

What was the warning?

There were several warnings. Basically, at compile time, it couldn't find some of the IBActions and IBOutlets defined in the code. But when I opened the .xib in IB, it showed them just fine.

Build Quicksilver of project Quicksilver with configuration Debug

CompileXIB Nibs/QSApplicationPrefPane.xib
/* com.apple.ibtool.document.warnings */
QSApplicationPrefPane.xib:-2: warning: The 'checkNow:' action of 'File's Owner' is connected to 'Push Button (Check Now)' but 'checkNow:' is no longer defined on 'QSApplicationPrefPane'.
QSApplicationPrefPane.xib:-2: warning: The '_firstKeyView' outlet of 'File's Owner' is connected to 'Check Box (Start at login)' but '_firstKeyView' is no longer defined on QSApplicationPrefPane.
QSApplicationPrefPane.xib:-2: warning: The 'runSetup:' action of 'File's Owner' is connected to 'Push Button (Run Setup)' but 'runSetup:' is no longer defined on 'QSApplicationPrefPane'.
QSApplicationPrefPane.xib:-2: warning: The 'uninstallQS:' action of 'File's Owner' is connected to 'Push Button (Uninstall Quicksilver)' but 'uninstallQS:' is no longer defined on 'QSApplicationPrefPane'.
QSApplicationPrefPane.xib:-2: warning: The '_window' outlet of 'File's Owner' is connected to 'Application' but '_window' is no longer defined on QSApplicationPrefPane.
QSApplicationPrefPane.xib:-2: warning: The '_initialKeyView' outlet of 'File's Owner' is connected to 'Check Box (Start at login)' but '_initialKeyView' is no longer defined on QSApplicationPrefPane.
QSApplicationPrefPane.xib:-2: warning: The 'resetQS:' action of 'File's Owner' is connected to 'Push Button (Reset Preferences)' but 'resetQS:' is no longer defined on 'QSApplicationPrefPane'.

CompileXIB Nibs/QSSetupAssistant.xib
/* com.apple.ibtool.document.warnings */
QSSetupAssistant.xib:-2: warning: The 'gettingStartedView' outlet of 'File's Owner' is connected to 'Web View' but 'QSUndraggableWebView' is not a kind of 'WebView' as specified by the outlet type.
QSSetupAssistant.xib:-2: warning: The 'gettingSupportView' outlet of 'File's Owner' is connected to 'Web View' but 'QSUndraggableWebView' is not a kind of 'WebView' as specified by the outlet type.

@skurfer
Copy link
Member

@skurfer skurfer commented Apr 24, 2012

OK. Well, your changes to the XIBs cause no problems in Xcode 4 that I can see. I’ll probably merge this later today.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Apr 24, 2012

I haven't had a chance to look over it (by all means do it Rob), but I just wanted to say this is good to see :)

I think we may have to do the 64 bit switch from scratch considering many of the changes here and in other places, but that shouldn't be too difficult.

skurfer added a commit that referenced this issue Apr 26, 2012
Reduce the number of compiler warnings
@skurfer skurfer merged commit 39d5af0 into quicksilver:master Apr 26, 2012
@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented May 7, 2012

whoever gets a chance to look at warnings next, I made a few changes here which could be either merged in or looked at.

Yes, I'm going through all my old branches...

@skurfer
Copy link
Member

@skurfer skurfer commented May 7, 2012

I was thinking this would be good to do at the same time the 64-bit changes are made. If not, I will work on it, but I’ll want to wait until 64-bit stuff is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants