From: Brandon Casey Date: Mon, 23 Sep 2013 17:21:15 GMT Subject: Re: [PATCH 00/15] Make Gnome Credential helper more Gnome-y and support ancient distros Message-ID: In-Reply-To: <20130923102022.GA96514@yoda.local> On Mon, Sep 23, 2013 at 3:20 AM, John Szakmeister wrote: > On Sun, Sep 22, 2013 at 10:07:56PM -0700, Brandon Casey wrote: >> A few cleanups, followed by improved usage of the glib library (no need >> to reinvent the wheel when glib provides the necessary functionality), and >> then the addition of support for RHEL 4.x and 5.x. >> >> Brandon Casey (15): > I reviewed all of the commits in this series, and most are nice > cleanups. The only thing I'm a little shaky on is RHEL4 > support (patch 15). In particular, this: > > +/* > + * Just a guess to support RHEL 4.X. > + * Glib 2.8 was roughly Gnome 2.12 ? > + * Which was released with gnome-keyring 0.4.3 ?? > + */ > > Has that code been tested on RHEL4 at all? I imagine it's hard > to come by--it's pretty darn old--but it feels like a mistake to > commit untested code. The (poorly worded) comment is referring to the version of glib that is being tested by the pre-processor statements. Since gnome-keyring doesn't provide a way to test its version, and I'm not sure exactly which gnome release included gnome-keyring 0.4.3 or which glib version was distributed with which gnome version, I'm just roughly guessing based on dates and not-conclusive google searches that 2.8 is reasonable. I'll try to clarify the wording here. The code has been tested on CentOS 4.X. > Otherwise, there are a few stylistic nits that I'd like to clean > up. Only one was introduced by you--Felipe pointed it out--and Well, not exactly "introduced" by me, but I guess I can fix it in that same patch. :) > I have a patch for the rest that I can submit after your series > has been applied. Sounds good. > Acked-by: John Szakmeister -Brandon