This function obviously belongs in Bugzilla::Group. However, generally, instead of using this function we should be creating Group objects. It takes about as much time to create a Group object as it does to call this function.
Also, the "GroupExists" function is used only once, and is the *exact same function* as GroupNameToId. So we just use group_name_to_id instead.
Created attachment 213023 [details] [diff] [review] v1 Once again, a very simple patch. :-)
Created attachment 213024 [details] [diff] [review] v2 I improved the POD slightly.
Comment on attachment 213024 [details] [diff] [review] v2 >Index: Bugzilla/Group.pm >=================================================================== >+=item C<group_name_to_id($name)> Nit: You could have also added this to the synopsis section in the POD. I wouldn't mind if the Bugzilla::get_all_groups would be corrected at the same time (should be Bugzilla::Group::get_all_groups). If you want, both can be fixed on checkin. >+ Description: Converts a group name to an id. >+ In general, instead of using this function, you should >+ create a Group object and get its name. This function >+ does not offer any real performance advantage. Makes me think this sub shouldn't exist but the three places where this is used be converted to Group objects.
Okay, did the checkin fix for the POD. Checking in editgroups.cgi; /cvsroot/mozilla/webtools/bugzilla/editgroups.cgi,v <-- editgroups.cgi new revision: 1.66; previous revision: 1.65 done Checking in editproducts.cgi; /cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v <-- editproducts.cgi new revision: 1.114; previous revision: 1.113 done Checking in globals.pl; /cvsroot/mozilla/webtools/bugzilla/globals.pl,v <-- globals.pl new revision: 1.353; previous revision: 1.352 done Checking in Bugzilla/Group.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Group.pm,v <-- Group.pm new revision: 1.11; previous revision: 1.10 done
Created attachment 213646 [details] [diff] [review] v3 (checked-in version) Okay, here's the version that I checked-in.