bug-gnulib
[Top][All Lists]
Advanced

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

unreviewed patches on master


From: Bruno Haible
Subject: unreviewed patches on master
Date: Sun, 6 Feb 2011 03:34:14 +0100
User-agent: KMail/1.9.9

Hi Bruce,

> I have merged the patch:
> http://lists.gnu.org/archive/html/bug-gnulib/2011-02/msg00044.html
> into the sources, yielding the patch below.  Barring final testing
> problems or complaints, I am hoping to push later today.

You pushed this already.

Paul said that he reviewed a tiny part of it (openat-die). The rest
was unreviewed.

Not only it was unreviewed, there was also no agreement on details of
the approaches. Gary started 7 discussion threads, and the last status
was here:
  <http://lists.gnu.org/archive/html/bug-gnulib/2010-12/msg00280.html>

There is also an 8th issue that you have not addressed (or at least you
have not answered in this thread): the versioning
  <http://lists.gnu.org/archive/html/bug-gnulib/2011-01/msg00163.html>

I concede that it is not easy to get my attention for review. Especially this
week, the following topics are still pending and need work:
  - "Relocation patch for cygwin"
  - "Document reasonable portability targets"
  - "16-bit wchar_t on Windows and Cygwin"
  - "Openat first step passfd api"
  - "take3: Add an implementation of gnulib's canonicalize_file_name for mingw."
  - "gnulib silently breaks putenv on mingw"
I regret not having been able to review your submission from Thursday in two
days. It was a busy week.

But that is not a reason for pushing it to 'master' unreviewed. Especially
since you know from the above past discussions that the topics are not
fully discussed.

If you could not find someone who reviews the part of the patch, aside from
'openat-die', you should have pinged people again. And not only me: It does
not always need me who reviews things. Paul and Eric and also good reviewers.
Although, for patches to 'gnulib-tool' and 'iconv-open', you definitely need
my agreement.

If I try to review what you committed, I have many objections. Here's only
the beginning:
  - The entries in ChangeLog should be moved to the top (at the moment of the
    commit to master), so that the ChangeLog reflects the order of the
    appearance of the patches in 'master'.
  - gnulib-tool: It breaks packages which have not initialized EXTRA_HEADERS.
  - gnulib-tool: As I said in a previous mail, only the header files of modules
    that were specified on the command line ought to be installed.
  - There is no reason to change modules/iconv_open.
  - and many more if I were to review the gnulib-tool changes.

I'll need at least 3-4 hours to review this. 

It's not an acceptable way of working like this. We need to review things in
smaller chunks. It would be ideal if you could take patches from the 'libposix'
topic branch, present them for review, let us rework them and then apply them
to 'master'.

You started like this with the test-fprintf-posix3 test. Yes it took a month
to get a good fix into 'master', but that's the only reasonable way of working.

So, please REVERT your commit to 'master', and start submitting the parts in
SMALL but self-contained pieces.

You can work as you wish on the 'libposix' branch, or create new branches if
you see fit. But for 'master', we need to work in small steps, with approaches
that have been discussed, and with patches that have been reviewed.

PS: The same rules apply to me too. My submitted patch for "Document reasonable
portability targets" is waiting because Paul and Simon did not like it this
way. But first pushing and then discussing is not an option.

PS2: I'm not saying that all of the patch needs to be done differently. Most
of the modules/* patches look right to me now (I disagreed two months ago),
but the gnulib-tool part that goes with it must be done differently.

Please REVERT.

Bruno
-- 
In memoriam Neil Aggett <http://en.wikipedia.org/wiki/Neil_Aggett>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]