GNOME Bugzilla – Bug 569890
Reduce/remove compiler warnings preparing for "warnings as errors"
Last modified: 2010-08-16 14:49:27 UTC
I'll abuse this bug to add/update patches that remove compiler warnings. I'm trying to keep them up to date and applicable to svn trunk. Patches will be grouped by error (unreachable code, hiding members, deprecated useage of 3rd party api etc.) to make the process of reviewing and applying them easier.
Created attachment 127582 [details] [review] Just missing a "new" to remove compiler warnings Low-hanging fruit..
Created attachment 127583 [details] [review] Unreachable code, fixes a small number of compile errors Again, nothing special
Created attachment 127584 [details] [review] Unused private fields, again fixing some compiler warnings This is a probably incomplete (and so might the other patches be), because I'm currently testing/reading on Windows and the VS solution doesn't include all files (lots of addins are missing for example). I'll update these as soon as my linux box is ready again.
Created attachment 127590 [details] [review] Deprecated API, causing some compiler warnings Some things are still missing. Apart from all the unchecked code that isn't included in the VS solution there's dead code causing a warning (TagEntry.cs) and a medium sized change pending in the synchronization implementation: Thread.Suspend()/.Resume() is long dead and should be replaced with a mutex/semaphore.
I removed a couple of no-longer-needed files you patched, and committed most of your changes in r2298. Some things had to be changed because they rely on #if blocks, etc. I still need to review that last patch.
Created attachment 130531 [details] [review] updated obsolete property of Gtk.AboutDialog about.Name = "Tomboy"; ---> about.ProgramName = "Tomboy"; to remove one compiler warning.
Thanks Eric. Please do not attach complete files, patches are easier for the maintainers to review. Here are some instructions for creating patches with 'svn diff': http://live.gnome.org/Tomboy/HowToSubmitPatches
Created attachment 130650 [details] [review] opdating obsolete Property updated this with the proper format of a patch. (thanks Stefan). Unfortunately this warning only shows up for me since I am on a newer version of Gtk. Maybe for the future.
Created attachment 135217 [details] [review] Fixing a lot of warnings, some whitespace issues Fixing a lot of warnings, some whitespace issues. Mostly removing Logger.Log calls and unused variables/obsolete gtk stuff. Reduces the warnings to roughly a dozen.
Some issues: * You got rid of instantiation of the TomboyPrefsKeybinder in Applet.cs and Tray.cs, which means keybindings go away. :-) You are correct that we don't need to keep a reference to it, though. * We can't get rid of "new" in TomboyApplet.SetupMenuFromResource, because that method exists in our bundled bindings. We would have to fix our bundled bindings or stop bundling them (meaning we would depend on gnome-sharp 2.24 or greater). * You removed a line that had a TODO above it. Note that there are some documented areas of Tomboy where we have to access a field to work around a bug. Make sure to investigate this one before removing it: // TODO: Is this variables used, or do we just need to // access iter.Tags to work around a bug? - Gtk.TextTag [] tags = iter.Tags; Otherwise, it looks good.
Created attachment 167961 [details] [review] Fixing all of the obsolete Log () warnings Late, but... Follow-up on the previous patch, removing the parts that were plain wrong and limited this patch to one purpose (obsolete .Log () calls) only.
Review of attachment 167961 [details] [review]: Beautiful, thanks!
Comment on attachment 167961 [details] [review] Fixing all of the obsolete Log () warnings Pushed to master