apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sander Striker <stri...@apache.org>
Subject Re: Hold Noses (was Re: svn commit: r157852 - in apr/apr-iconv/trunk: CHANGES lib/api_version.c lib/iconv_module.c)
Date Thu, 17 Mar 2005 09:24:32 GMT
William A. Rowe, Jr. wrote:
> At 09:57 PM 3/16/2005, Justin Erenkrantz wrote:
>>On Thu, Mar 17, 2005 at 03:16:44AM +0100, Sander Striker wrote:
>>>API_STRINGIFY?  When did we grow that?  Is this materially different than 
>>I know, I know.  OtherBill's patches required us to remove any dependencies
>>upon APR_* functions because the Win32 RC compiler can't handle it.  I held my
>>nose during the commit...  -- justin
> functions.  That's an include.  Wasn't required, simply trivial.
> However it was rather pointless to include apr for a two line #define,
> don't you think?  Apparently not.

Duping is always questionable.  We can simply add a comment like:

  /* Duping the APR_STRINGIFY macros here, because the Win32 RC compiler
   * doesn't like includes in the way we set things up.

End of story.

> If you have an issue, rather than grabbing a tissue, try addressing
> it to the list at that time, as opposed to personal snipes.

I assume everyone lets minor issues slide once in a while, weighing the
energy you have to spend on it against the desired result.
> Once again you sent me hunting for an [annotated] viewsvn history to
> track down which patch I added that in.

Noone is sending you hunting, that is something you decide to do on
your own.

> Oh wait - svn can't do that. Score another for premature adoption.

Ok, here it would have been best for you to grab a tissue.

Have you even tried running svn blame (or if you don't like that name:
svn praise)?  Or if you're a TortoiseSVN user tried running Blame from
the context menu?

Annotation is currently disabled in viewcvs.  It does work, but there
are some performance issues with it.  There are other ways to get to
the information though.

Let me show you:

  svn praise http://svn.apache.org/repos/asf/apr/apr-iconv/trunk/include/api_version.h

  157664 jerenkrantz #ifndef API_STRINGIFY
  157664 jerenkrantz /** Properly quote a value as a string in the C preprocessor */
  157664 jerenkrantz #define API_STRINGIFY(n) API_STRINGIFY_HELPER(n)
  157664 jerenkrantz /** Helper macro for API_STRINGIFY */
  157664 jerenkrantz #define API_STRINGIFY_HELPER(n) #n
  157664 jerenkrantz #endif

Ah, rev 157664.  What was that commit?

  svn log -r 157664 -v

  r157664 | jerenkrantz | 2005-03-16 05:25:48 +0100 (Wed, 16 Mar 2005) | 3 lines
  Changed paths:
     M /apr/apr-iconv/trunk/CHANGES
     M /apr/apr-iconv/trunk/include/api_version.h
     M /apr/apr-iconv/trunk/libapriconv.dsp
     A /apr/apr-iconv/trunk/libapriconv.rc

  Fix libapriconv.rc for Win32 release builds.
  Also sync up CHANGES and bump trunk to 1.1.0-dev.


Hmm, let's look at the CHANGES entry before we entirely praise jerenkrantz.

  svn diff -r 157663:157664 http://svn.apache.org/repos/asf/apr/apr-iconv/trunk/CHANGES

  Index: CHANGES
  --- CHANGES	(revision 157663)
  +++ CHANGES	(revision 157664)
  @@ -1,5 +1,13 @@
  -Changes with APR-ICONV 1.0
  +Changes with APR-ICONV 1.1
  +Changes with APR-ICONV 1.0.2
  +  *) Fix libapriconv.rc for Win32 builds [William Rowe, Justin Erenkrantz]
  +Changes with APR-ICONV 1.0.1
  +Changes with APR-ICONV 1.0.0
     *) Add the possiblity of a DESTDIR prefix to Makefile.in to make it
        consistent with the behaviour of apr and apr-util. [Graham Leggett]

So, this must have had some discussion on list somewhere before the commit.
The log said it was committed 16th of march, so lets have a look


[...stops looking for the mystery patch and starts packing...]


View raw message