[Aptitude-devel] Review: bf48d "Add tab classes and stubs of the program's tabs"

[Daniel Burrows]

  Review of bf48dae780fed461f37e5609122e92cf43b10191:

----- cut here -----
+      /** \brief tab is a base class for tabs shown is main window's tab widget.

  "is main window's tab widget" -> "in ..."

+       *  This class contains common methods and variables that all tabs have to
+       *  implement.

  Personally, I would leave this out; it's redundant with "base class".

+       *  Every tab is distinguished by name and has one of the supported type.

  Has one what of the supported type?

  How about "every tab has a name, which will be displayed in the UI,
and a type, indicating which of the UI pages it implements."

+	/** \brief Indicates package type */

  Shouldn't that be tab type? :)

  /** \brief Represents the type of a tab. */

+	enum tab_type
+	{
+	  tab_packages,
+	  tab_info,
+	  tab_preview,
+	  tab_resolver,
+	  tab_perform
+	};

  I think this is the right design: it lets other modules change their
behavior based on which tab is shown and keeps code that should be in
one place in one place.  Thank you for *not* using polymorphism here.

+	/** \brief Retrieve the description of this tab.
+	   *
+	   *  Description is used as a tooltip for given tab

  Change to "The description is used as the tooltip of the tab."
----- cut here -----

  I think I'm ready to accept this one with the changes above.

  Daniel