On 27-03-2012 12:51, Martin Nyhus wrote:
> On Monday 26. March 2012 00:22:24 Marco Schulze wrote:
>> Working (but incomplete) code is available @
>> https://github.com/Heiral/fred-staging/tree/logger++
> Please keep in mind that simply deleting Logger will break pretty much every
> single plugin out there, so you really should rewrite Logger to call the new
> methods in your new logger class.
Will do.

>
> I won't say much about the code since you say you aren't finished, but please
> follow the code style of the rest of the code base.
Apart from the lack of braces, what violates the coding standards? I 
mean, compared to the rest of fred code, I use too many blank lines, 80 
character lines, variables are declared at the top of the function and 
other minor details. I hope that that isn't a problem.

>
> Also, I'm pretty sure you don't want to close the new stream here[0].
Fixed.

> And the
> locking when you update out isn't sufficient for visibility (either that or it
> isn't clear enough why it works IMHO).
You mean, things like if(out == null) without locking (inside 
isLoggable())? In this case, it doesn't really matter as it is meant to 
be a very quick check and getting the wrong values don't matter much.

>
> [0] https://github.com/Heiral/fred-
> staging/commit/b876f8c454cb269281ffcb18d14d25b22818d130#L0R49
> _______________________________________________
> Devl mailing list
> Devl at freenetproject.org
> https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl

Reply via email to