Last Comment Bug 345552 - Remove a pre-panther check
: Remove a pre-panther check
Status: RESOLVED FIXED
: fixed1.8.1
Product: Camino Graveyard
Classification: Graveyard
Component: General (show other bugs)
: Trunk
: PowerPC Mac OS X
-- trivial (vote)
: Camino1.5
Assigned To: Peter Jaros (:peeja)
:
:
Mentors:
Depends on:
Blocks: 311840
  Show dependency treegraph
 
Reported: 2006-07-21 19:25 PDT by froodian (Ian Leue)
Modified: 2006-08-23 15:28 PDT (History)
1 user (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
removes old panther checks, SearchToolbarItemIdentifier, mBookmarkToolbarItem, and mSidebarToolbarItem in BWC (9.35 KB, patch)
2006-08-14 16:15 PDT, Peter Jaros (:peeja)
nick.kreeger: review+
mikepinkerton: superreview+
Details | Diff | Splinter Review
This is the bitrot police (9.36 KB, patch)
2006-08-23 13:47 PDT, froodian (Ian Leue)
no flags Details | Diff | Splinter Review

Comment 1 User image Smokey Ardisson (offline for a while; not following bugs - do not email) 2006-08-13 23:21:56 PDT
Let's get a patch up for this and get it off the list ;)
Comment 2 User image Peter Jaros (:peeja) 2006-08-14 16:15:02 PDT
Created attachment 233668 [details] [diff] [review]
removes old panther checks, SearchToolbarItemIdentifier, mBookmarkToolbarItem, and mSidebarToolbarItem in BWC

Cleaned up a lot of old code.  Not just the Panther checks, but also references to SearchToolbarItemIdentifier that are now defunct, and mBookmarkToolbarItem and mSidebarToolbarItem, since they were not only redundant, but never used.
Comment 3 User image Smokey Ardisson (offline for a while; not following bugs - do not email) 2006-08-14 20:57:52 PDT
>  // toolbarWillAddItem: (toolbar delegate method)
>  //
>  // Called when a button is about to be added to a toolbar. This is where we should
> -// cache items we may need later. For instance, we want to hold onto the sidebar
> -// toolbar item so we can change it when the drawer opens and closes.

Won't that still be true (be needed) when we get the Bookmarks toolbar button changing state depending on the state of the Manager?  Or are we "holding on" to it for another reason entirely there?
Comment 4 User image Peter Jaros (:peeja) 2006-08-15 12:30:32 PDT
Well, mBookmarkToolbarItem and mSidebarToolbarItem are never read from in the entire project, and the changes seemed not to break the logic that chages the Bookmarks button's state.  The state changing should be handled by validateToolbarItem:.
Comment 5 User image Nick Kreeger 2006-08-15 15:00:27 PDT
Comment on attachment 233668 [details] [diff] [review]
removes old panther checks, SearchToolbarItemIdentifier, mBookmarkToolbarItem, and mSidebarToolbarItem in BWC

+// cache items we may need later.
+// (void)toolbarWillAddItem:(NSNotification *)notification
+//{
+//  (Nothing needed at the moment.)
+//}

Whats the point of keeping this?

Looks OK, I doubt that this is all the panther checking code that we have.
Comment 6 User image Peter Jaros (:peeja) 2006-08-16 09:26:41 PDT
(In reply to comment #5)

> Whats the point of keeping this?

I'm happy to get rid of it, but in IRC people said to be wary of removing methods.  Since the method would be empty, I thought this was a compromise. :)

Of course I'm happy to defer to a higher ruling.
Comment 7 User image Mike Pinkerton (not reading bugmail) 2006-08-21 13:47:15 PDT
Comment on attachment 233668 [details] [diff] [review]
removes old panther checks, SearchToolbarItemIdentifier, mBookmarkToolbarItem, and mSidebarToolbarItem in BWC

sr=pink

i just want to make sure this doesn't break people upgrading from 1.0 where that search field key is used in the toolbar plist.
Comment 8 User image froodian (Ian Leue) 2006-08-23 13:47:57 PDT
Created attachment 235133 [details] [diff] [review]
This is the bitrot police

I tested this out going from a clean 1.0 profile to a build with this patch in it with no ill consequences.  It had however, bitrotted (from my whitespace fixes).  Since peter is sadly bereft of a machine capable of building, I'm unbitrotting for him.
Comment 9 User image Håkan Waara 2006-08-23 15:28:40 PDT
Checked in.

Note You need to log in before you can comment on or make changes to this bug.