[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Comments on xerces-c patch please

On Fri, Aug 05, 2011 at 09:21:04AM +0200, Svante Signell wrote:

> Why didn't you update the patch yourself instead of complaining on my
> patch, or at least give a link to working code. Currently I don't know
> how to write this test, and after your reply I don't know either.
> Constructive criticism is always better than destructive.

Guillem apparently assumed you know how to write such a test. And I’m
pretty sure that, since that is not the case, he will be glad to show
you how to write it. In the process, you will acquire a new skill, one
that you probably would not have learned if he had just went ahead and
updated the patch for you.
> > For the indentation (spaces/tabs) I'd recommend you enable something
> > like this in your editor, vim in this case but emacs and other editors
> > should have an equivalent:
> > 
> >   let c_space_errors=1
> >   set listchars=tab:»‐,trail:·,extends:…,nbsp:‗
> >   set list
> This is constructive, but unfortunately I don't use vim and have no idea
> what this means in Emacs terms.

According to [1], the first one will mark unwanted spaces in red when
you’re editing a C file, while the second and the third options will
graphically show whitespace characters, using » for tabs, · for trailing
whitespace and so on. This is meant to make detecting it easier.

Now that you know what it does (I didn’t either, but it only took me five
minutes of Googling to find out) you can try and find a suitable Emacs

> > For the blank lines, coding style etc, you should review your own
> > patches before sending them.
> Why having reviews at all then, if everything is perfect by definition.
> No more updated patch from me this time, maybe you can take over and
> make it perfect. I will consider if I'm going to continue struggling
> with GNU/Hurd. There are better ways to spend your _free_ _unpaid_ time,
> instead of being criticised when trying to contribute. 
> How many patches per day are sent by the very few GNU/Hurd developers?
> After following the lists and #hurd for some time now my impression is
> that most communication is words mainly, not many lines of code are
> produced.

Why having reviews at all then, if you don’t want to accept criticism?

I think Guillem did the right thing here: he explained you exactly why
your patch is not ready for submission yet, and how you can make it so.
If any of the details is not clear to you, I’m willing to bet he’ll be
more than happy to give you further explanation and guidance.

It’s true, that will make for more work on your side; on the other hand,
next time you’re working on a patch, you will probably take care not to
include spurious whitespace and to respect upstream’s coding standards,
so that whoever reviews it will be able to concentrate on the patch’s
juice part only.

It’s not about being picky, either: most upstream developers will refuse
to apply a patch if it does not respect the coding standards, and will
not bother rewriting it themselves unless it brings a whole lot of
useful functionality. Guess what? Most people don’t care about GNU/Hurd
at all, that’s why we need the patches we submit to be as small as we
can. Everything else is just standard pratice when it comes to having
one’s patches applied upstream.

Don’t take it personally when someone finds some problem with your
patch: they just want the patch to be good. As you fix your patches
you’ll learn a lot of useful stuff, and if you don’t give up you’ll
be flying through the review process in no time!

[1] http://vim.wikia.com/wiki/Highlight_unwanted_spaces
Andrea Bolognani <eof@kiyuko.org>
Resistance is futile, you will be garbage collected.

Attachment: signature.asc
Description: Digital signature

Reply to: