GNOME Bugzilla – Bug 620402
Show addin version and categories tree in addin preferences tab
Last modified: 2010-12-30 20:46:09 UTC
Created attachment 162570 [details] [review] Feature implementation. Implementation of addin preferences tab, more similar to Tomboy one: * Addins are shown in tree of categories. * Show addin version, instead of description.
Created attachment 166773 [details] [review] Feature implementation. Improved patch to show package icon and grey name for disabled addins.
Created attachment 171450 [details] [review] Version using addin categories constants and supporting categories localization. Updated version of patch, requires patch with turning addin categories into constant identifiers in Bug 621827 to be committed.
Review of attachment 166773 [details] [review]: The other solution is chosen.
Created attachment 172644 [details] [review] Version using addin categories constants and supporting categories localization. Updated patch, to use the right constants.
Comment on attachment 172644 [details] [review] Version using addin categories constants and supporting categories localization. AddinsTreeModel::get_addin_category_name() should be a static method. Also I don't think I agree on changing the value of its parameter and using "Tools" as a default category.
(In reply to comment #5) > (From update of attachment 172644 [details] [review]) > AddinsTreeModel::get_addin_category_name() should be a static method. Good point. I will fix that. > Also I don't think I agree on changing the value of its parameter and using > "Tools" as a default category. This was done for the case, when addin returns unknown category. Maybe we should create a separate category "Other" or "Unknown" for them? It's not nice, if they all fall into separate categories without names. Changing parameter was done, because I use the integer value to identify the tree item, to which the addin should be placed, so it seemed to me like the most straight way do it.
Created attachment 172955 [details] [review] Version using addin categories constants and supporting categories localization. Added new addin category `unknown', which is used as default fallback. get_addin_category_name changed to be public static method. Added public static method ensure_valid_addin_category, to make sure the addin category is one of our constants. The rest is updated to use the above.
Review of attachment 172955 [details] [review]: Beside the "curly braces are mandatory" and "nothing allowed on the same line after an else but another if()" rules, the patch looks fine to me.
I've fixed the noted issues and pused this patch.