Re: [Cdk-devel] Patch for exception handling in builder3d

[Stefan K. <ste...@eb...>]

What I main with meaningless comments is
@exception CDKException  Description of the Exception
Before this was
@exception Exception  Description of the Exception
If I want to make this more meaningfull, I have to look through which methods 
are called and when they might be thrown etc. and this is something I don't 
want to do. I think having the more specific exceptions is an improvment, but 
if you don't think so, I am happy to discuss this.
Now you say "Every line you change, you take responsibility for". I changed 
the comments line since Exception became CDKException. So taking you literal, 
I would have to provide proper comment. And this is what I don't want to. I 
improved the exception handling (again, if it's not an improvement, I am 
happy to change the patch), but I don't think because I did this 
improvements, I have to improve the comment as well (and perhaps other 
stuff).
If you agree on this, I am fine. If not, I see a problem. Every improvment I 
do will make you realize another 10 problems and if I am then blamed for 
these, I cannot see how I can work on cdk except bits I did myself totally 
from scratch.
Stefan

On Friday 24 April 2009 11:38:15 Egon Willighagen wrote:
> Stefan,
>
> On Thu, Apr 23, 2009 at 7:26 PM, Stefan Kuhn <ste...@eb...> wrote:
> > But please: Don't tell me the comments are meaningless and other stuff. I
> > know that, but just that I did this bit does not mean I take over
> > responsibility for everything in these classes.
>
> Every line you change, you take responsibility for. Every method you
> change you also take responsibility for, but only if the code is
> changed. If you change code, you *cannot* say you "I don't care if it
> works or not". That is not how things work.
>
> What you do *not* take responsibility for is the code written by
> other, but I cannot live in a world where people make modifications to
> things and not caring about if the changes make sense.
>
> Also, I do expect you to report problems in other peoples code, when
> you modify that code. You do take responsibility for that too.
>
> If you do not like replying the people reviewing *your* code, then
> don't provide patches. No one will hold you responsible for bugs in
> other people's code, but when you make changes, you simply do take
> responsibility for what you do. If you clean up code, people may
> expect you to do that properly. Jobs half finished are a waist of time
> for reviewers, and other people trying to understand the code. Every
> now patch, after all, adds complexity to the code history and to the
> understanding of what the code is meant to do.
>
> I am not sure what 'meaningless comments' you refer too, but such
> comments added by you, you can expect complaining about. If they were
> not added by you, you should generally be safe, except perhaps for
> only having cleaned up the code half.
>
> In short, your responsibility does go beyond the lines you changed
> yourself, but certainly does not extend to all code in the class. If
> you do not like that, then do not touch the code, and just file bug
> reports, or send a code review report via one of the channels.
>
> We are all in the same situation here, we all have to reply to
> comment. Please stop complaining about it.
>
> Egon



-- 
Stefan Kuhn B. Sc. M. A.
Software Engineer in the Chemoinformatics and Metabolism Team
European Bioinformatics Institute (EBI)
Wellcome Trust Genome Campus
Hinxton, Cambridge CB10 1SD UK
Phone +44 1223 49 2657
Fax +44 (0)1223 494 468