Hi Vivia, Cristofaro, everyone else...
First, Cristofaro, sorry for not committing the patch earlier, I started
looking at it and didn't have time to
finish reviewing the code. I just starting looking at it again when I saw vivia
committed it already. So again,
sorry for the slow response from my side.
Vivia, you screwed up big time! As you should know, we must never commit any
patch to SVN without prior checking
and reviewing the code. You just told me that you didn't have time to review
the code, I understand, and you
didn't want the thread to go unnoticed, I understand, but you know me, I rarely
forget about some thread and I
was surely going to do this (especially since Cristofaro is in my MSN list, so
everytime I look at my CL I
remember the patch). Also, you should have just answered to say "hey, don't
forget about this patch.." or
something.
I will know review all that code, I don't want any more commits without knowing
what the code contains. It's not
because I don't trust the code, it's just that whenever there is new code, it's
better to review it to make sure
it's good code, no hacky code, nothing that gets forgotten.. as you should
know, there were many commits that
were done with some small typos, lower/upper case, or a == 0 instead of != 0,
etc.. and even if the code works,
the bug may not arise because you can't test all cases, and the best way to
catch the bugs, is as early as
possible, which means, before the commit (or right after the commit when
looking at the amsn-commits ML). In
this case, I'd like to review the code to make sure there's nothing that was
forgotten.
Cristofaro is also new to amsn and to Tcl, so there are many things he doesn't
know because of experience, for
example, a bind $w <1> "set_display_pic $filename" is something that might work
and he may not see the problem
with it, but actually, it's a problem in case there's a space, because Tcl will
evaluate this and make spaces in
$filename as multiple arguments. the best way to fix it is doing bind $w <1>
[list set_display_pic $filename].
Those are tricks we got to know by experience and it took a lot of time to
figure those out.
Same for if we see [expr $x * 2] that's wrong, it should be [expr {$x *2}]..
it's the same thing, but the second
form will avoid the Tcl core to go through some processing that will make the
expr evaluate with an increased
performance.. etc..
I'll answer back again once I review the code.. and I may hopefully be able to
commit fixes to it too.
KKRT
On Mon, Jan 08, 2007 at 04:41:06PM +0200, Vivia Nikolaidou wrote:
>
> Thanx, committed :)
>
> On Mon, 8 Jan 2007, Cristofaro Del Prete wrote:
>
> > Vivia Nikolaidou ha scritto:
> > > It's OK, but since I committed your previous patch already, can you diff
> > > against current SVN and resend it? :)
> >
> > I checked out amsn-devel before of amsn-commits... sorry
> > Patch attached to message.
> >
>
> -------------------------------------------------------------------------
> Take Surveys. Earn Cash. Influence the Future of IT
> Join SourceForge.net's Techsay panel and you'll get the chance to share your
> opinions on IT & business topics through brief surveys - and earn cash
> http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
> _______________________________________________
> Amsn-devel mailing list
> Amsn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/amsn-devel
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Amsn-devel mailing list
Amsn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/amsn-devel
