On Sat, Jun 20, 2009 at 12:31 AM, Jakub Narebski<jnareb@xxxxxxxxx> wrote: >>> Question: why it is not overridable (why project specific override >>> is not supported for this feature)? Some projects may use Gravatars, >>> some might not, although I guess that usually it is deployment specific >>> feature. >> >> I see it as a deployment feature, and considering that it adds an >> (admittedly small) extra load on the server, I thought it was sensible >> to make it non-overridable. OTOH, since the load is small, it might be >> possible to make it per-project without big issues. > > Well, I was not asking you to change it; I was asking about > justification behind making it non-overridable. I decided to go for the override because it was little effort and it whether or not the override can take place can still be decided at a central place. >>> You would probably want to protect against $size being undefined: >>> >>> + $size = 32 if (!defined($size) || $size <= 0); >>> >>> Because currently when you are not passing size parameter to use >>> default size you would get the following warning: >>> >>> Use of uninitialized value in numeric le (<=) ... >> >> Oh right. > > Well, as you use 'undef' (do not pass parameter) for default value > and you _do not_ use negative (or zero) value, then > > + $size = 32 if !defined($size); > > would be enough (in Perl 6 / Perl 5.10 it would be "$size //= 32" ;-) > > The question is: does gravatar.com accepts any size? What does it do > if it gets negative size passed? I have not used gravatar as > developer... If you pass a negative size, the image comes up blank. Using undef for the default value makes sense. The question rather becomes whether it makes sense or not to provide a safety fallback for when the size comes up negative. >>> Also gravatars are >>> not shown in 'history' view, but I guess that could wait for proper >>> refactoring of all log-like views/actions to use common infrastructure. >> >> That's part of the reason, the other being that I couldn't find a >> satisfactory way to do it 8-P > > What is the problem? I really don't remember, I created the patch a long time ago. As you can probably seen from v2, I just decided to go with the same code everywhere (which asks for a refactoring). >>> Those all look nice with the *default* font sizes. But as the size of >>> gravatar is used when constructing gravatar URL, to pass to gravatar.com >>> I don't see how this problem can be resolved... Beside making it >>> configurable, I guess... >> >> That's something I hadn't thought about, honestly. The problem of >> course is that the font sizes get customized via CSS, but the gravatar >> size would get customized at the cgi level ... so unless we parse the >> CSS from the cgi it cannot be done automatically. > > What I though here was to use %gravatar_size hash, with keys such as > 'default' and 'double' (for "double line height"). If you change or > add CSS, changing configuration, you can change also this in config. And since the sices could be used for other things than gravatar, we could call it just %avatar_size_hash or whatever, and future support for picons or whatever else can use it too. -- Giuseppe "Oblomov" Bilotta -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html