[prev in list] [next in list] [prev in thread] [next in thread] 

List:       koffice-devel
Subject:    Re: Revert in KWord
From:       Inge Wallin <inge () lysator ! liu ! se>
Date:       2010-08-11 7:11:34
Message-ID: 201008110911.34948.inge () lysator ! liu ! se
[Download RAW message or body]

On Tuesday, August 10, 2010 17:51:47 zander@kde.org wrote:
> On Tuesday 10. August 2010 16.29.17 Inge Wallin wrote:
> > On Tuesday, August 10, 2010 16:12:13 zander@kde.org wrote:
> > > I just noticed this commit email message;
> > > 
> > > This revert of my work has not been announced or discussed anywhere, no
> > > breakges have been reported on bugzilla or anywhere else I could find.
> > > 
> > > Can we please communicate instead of commit reverts of others work? I
> > > think this specific request has been made a LOT in the last couple of
> > > weeks so this revert really surprises me in a bad way :-(
> > 
> > I know the commit message said "revert", but you should look at it as a
> > fix.
> 
> The patch reverts various fixes I made in document model and stability.
> Reverting my work most definitely needs more than a "its a fix!", I might
> actually have knowledge about this stuff, you know. I wrote most of it ;)

Ok, I'm not qualified to comment on the actual content of the patch.  However, 
the fix (or revert if you prefer) is short (22 lines including all additions 
and removals) and is no way near the size of your original patch.  So it may 
be true that it does revert various fixes, but few in comparison to what whas 
done in your own commit.  I know for a fact that Matus was careful not to 
revert more than necessary.

For the actual effect of the fix, it did revert a glaring problem, namely that 
the first page of a multi-page document was always empty.  Perhaps your fixes 
increased stability, I don't know, but to do that to the cost of not showing 
any contents is probably wrong.  :-)  

I know that you know this code, and that you wrote it.  However, I also know 
that nobody is perfect.  This time you just happened to screw up.  It happens 
to us all, it's not a big deal.  But what I think *is* a big deal is to berate 
somebody when he fixes your mistake and in the process tries to minimize the 
impact.  Why not just say "ok, this part was wrong, I'll try to redo the 
improvements but this time without the bad side effects."?

> > Your commit did seriously break text painting in a way that made the
> > white background be painted above the text in a lot of documents.  This
> > is the only part of your work that was "reverted", i.e. fixed.
> 
> Then communicate this. I think everyone here will agree that undoing work
> done by others is impolite at best. Undoing the work of the maintainer
> double so.

You are "undoing" other people's work all the time.  This may sound offensive, 
but the way you use "undo" here, it actually means "improve".  Your code was 
mostly good, but this small part was not.  So it was fixed.  I don't think 
that's a bad thing, do you? 

> > > And let me be crystal clear; in KWord I am the maintainer and I very
> > > much welcome people working together to fix problems and add features.
> > > The way that this happens is by talking to me and not working around
> > > me. If you try this anyway you will notice your fixes no longer are
> > > welcome because I value cooperation and a friendly atmosphere much
> > > more than I value code.
> > 
> > I think fixing an obvious bug with a small fix is in fact cooperating
> > just fine.
> 
> My "Crystal clear" is not very clear then. :-(
> Undoing work I did because you think its wrong by just reverting it and not
> explaining what was wrong is impolite and just wrong. And this is not
> limited to KWord, this is a KDE wide policy.
> 
> If you think I did something wrong you either understand my intention and
> fix it so both our ways work, or you talk to me.
> What Hanzes did was just remove my fixes since they seem to have interfered
> with his work. And you call it a fix. I disagree. This is not cooperation.
> 
> Thanks fly to Hanzes for emailing me the actual problem (see other mail on
> this thread) he is seeing, so lets not discuss this further and instead
> start cooperating. :-)

Ok, let's do that.  But with all that said, I would like to add one thing.  It 
was very hard to understand your intention with the fixes because they were 
never stated anywhere.  Can we all please start to comment our code so that 
things like this doesn't happen again?  A good comment always tells *why* 
something is done the way it is, not *what* it does.  The latter should be 
obvious from the code itself, at least in 95% of the cases.  If there had been 
some comments like this, maybe Matus had been able to fix the problem (no 
contents on page 1) while still keeping your improvements.
_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel
[prev in list] [next in list] [prev in thread] [next in thread]