[aur-dev] [PATCH] Add a logging function and logging fixins through the code

[Callan Barrett]

On Thu, Jul 2, 2009 at 1:51 AM, Dan McGee<dpmcgee at gmail.com> wrote:
> On Wed, Jul 1, 2009 at 12:42 PM, Sebastian Nowicki<sebnow at gmail.com> wrote:
>>
>> On Jul 1, 2009, at 7:36 PM, Callan Barrett wrote:
>>
>>>
>>> On 02/07/2009, at 1:31 AM, Sebastian Nowicki wrote:
>>>
>>>>
>>>> Might be easier to just put the "if(LOGGING) {}" into the logit()
>>>> function itself, assuming this is the desired behavior for all instances of
>>>> logging.
>>>
>>> I feel like an idiot for not putting it there in the first place but now
>>> that I think about it... if logging was turned off we would be doing 2 hits
>>> to the DB for almost every single action needlessly if the check was inside
>>> the function. So maybe it's not worth the simplicity?
>>
>> Ah, you mean the calls to username_from_sid() and pkgname_from_id()? If they
>> are always called, you could put that in the function as well, and only pass
>> in the parameters ($_COOKIE['AURSID'], $pid), respectively, etc.). Otherwise
>> you have a good point.
>>
>> The current interface for the logit function is better (more flexible) than
>> with the above suggested changes though. I'd probably stick with the
>> redundancy.
>
> I'd move the two function calls into logit() as well, as it looks like
> they are always called.
>
> Why are we inserting username when userid should be the foreign key we
> use to reference users? I guess if you aren't a fan of joins it makes
> sense, but DB normalization does have benefits, especially when you
> decided to use integers anyway to represent the different operations.
>
> The same holds true for package IDs, why not just use the IDs directly
> and not names? Getting the same end result table (with usernames and
> pkgnames) would be only a matter of two simple joins, not hard for a
> relational DB to do.
>
> In addition, magic numbers are not so much fun and make it hard to
> decipher the calls at first glance. Maybe some sort of associative
> array could help here, declared near the logit() function?
>
> $logit_actions = array(
> 'upload'=>1,
> 'update'=>2,
> ...
> );
>
> And then a call would end up looking like:
> logit(logit_actions('upload'), $userID, $packageID);
>
> -Dan
>

When I wrote this it was just gonna be a slapped together function we
use to get some stats before the planned rewrite. You've made me feel
guilty for not putting any thought into it now, I must rewrite this
patch!

-- 
Callan Barrett