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 equivalent. > > 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