You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ben Gollmer <be...@jatomail.net> on 2004/02/15 01:11:29 UTC

[PATCH] Add verbose support to blame

I was reading the thread 'annotate support for emacs', and I thought 
I'd give this patching thing a shot. Yes, the date format is verbose, 
but that's the way libsvn_subr/time.c said to do it.

Log:
* clients/cmdline/main.c: Add '-v | --verbose' switch support to
     svn blame.
* clients/cmdline/blame-cmd.c
     (svn__cl_blame): Pass the verbose argument to svn_client_blame().
     (blame_reciever): If the verbose argument is present, format the
     date and include it in the output.
* include/svn_client.h: Add 'svn_boolean_t verbose' argument to
     the declaration of svn_client_blame().
* libsvn_client/blame.c
     (svn_client_blame): Pass the verbose argument to blame receiver.

Index: subversion/include/svn_client.h
===================================================================
--- subversion/include/svn_client.h	(revision 8640)
+++ subversion/include/svn_client.h	(working copy)
@@ -332,6 +332,7 @@
                                  const char *author,
                                  const char *date,
                                  const char *line,
+                                svn_boolean_t verbose,
                                  apr_pool_t *pool);


@@ -752,6 +753,7 @@
  svn_client_blame (const char *path_or_url,
                    const svn_opt_revision_t *start,
                    const svn_opt_revision_t *end,
+                  svn_boolean_t verbose,
                    svn_client_blame_receiver_t receiver,
                    void *receiver_baton,
                    svn_client_ctx_t *ctx,
Index: subversion/libsvn_client/blame.c
===================================================================
--- subversion/libsvn_client/blame.c	(revision 8640)
+++ subversion/libsvn_client/blame.c	(working copy)
@@ -339,6 +339,7 @@
  svn_client_blame (const char *target,
                    const svn_opt_revision_t *start,
                    const svn_opt_revision_t *end,
+                  svn_boolean_t verbose,
                    svn_client_blame_receiver_t receiver,
                    void *receiver_baton,
                    svn_client_ctx_t *ctx,
@@ -567,7 +568,7 @@
            if (!eof || sb->len)
              SVN_ERR (receiver (receiver_baton, line_no, 
walk->rev->revision,
                                 walk->rev->author, walk->rev->date,
-                               sb->data, iterpool));
+                               sb->data, verbose, iterpool));
            if (eof) break;
          }
      }
Index: subversion/clients/cmdline/blame-cmd.c
===================================================================
--- subversion/clients/cmdline/blame-cmd.c	(revision 8640)
+++ subversion/clients/cmdline/blame-cmd.c	(working copy)
@@ -22,6 +22,7 @@
  #include "svn_error.h"
  #include "svn_pools.h"
  #include "svn_cmdline.h"
+#include "svn_time.h"
  #include "cl.h"

  
@@ -33,14 +34,27 @@
                  const char *author,
                  const char *date,
                  const char *line,
+                svn_boolean_t print_date,
                  apr_pool_t *pool)
  {
    svn_stream_t *out = baton;
+  apr_time_t atime;
+  const char *time_utf8;
+  const char *time_stdout;
+
+  if (print_date)
+  {
+    SVN_ERR (svn_time_from_cstring (&atime, date, pool));
+    time_utf8 = svn_time_to_human_cstring (atime, pool);
+    SVN_ERR (svn_cmdline_cstring_from_utf8 (&time_stdout, time_utf8, 
pool));
+  }
+
    const char *rev_str = SVN_IS_VALID_REVNUM (revision)
                          ? apr_psprintf (pool, "%6" SVN_REVNUM_T_FMT, 
revision)
                          : "     -";
-  return svn_stream_printf (out, pool, "%s %10s %s\n", rev_str,
-                            author ? author : "         -", line);
+  return svn_stream_printf (out, pool, "%s %10s %s %s\n", rev_str,
+                            author ? author : "         -",
+                            print_date ? time_stdout : "", line);
  }


@@ -100,6 +114,7 @@
        err = svn_client_blame (target,
                                &opt_state->start_revision,
                                &opt_state->end_revision,
+                              opt_state->verbose,
                                blame_receiver,
                                out,
                                ctx,
Index: subversion/clients/cmdline/main.c
===================================================================
--- subversion/clients/cmdline/main.c	(revision 8640)
+++ subversion/clients/cmdline/main.c	(working copy)
@@ -161,7 +161,7 @@
      "Output the content of specified files or\n"
      "URLs with revision and author information in-line.\n"
      "usage: blame TARGET...\n",
-    {'r', SVN_CL__AUTH_OPTIONS, svn_cl__config_dir_opt} },
+    {'r', 'v', SVN_CL__AUTH_OPTIONS, svn_cl__config_dir_opt} },

    { "cat", svn_cl__cat, {0},
      "Output the content of specified files or URLs.\n"


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: [PATCH] Add verbose support to blame

Posted by kf...@collab.net.
Ben Reser <be...@reser.org> writes:
> You changed the function signature.  So this isn't going to go into
> 1.0.x.  You'd have to make a svn_client_blame_verbose() to put it in
> 1.0.x.  

I think you mean 1.1.x, not 1.0.x?

Also, we should emphasize that it's not the end of the world if a
patch contributor isn't aware of Subversion's release compatibility
guidelines (Ben G, see the HACKING file if you're curious).  We can
easily take care of trivial things like renaming the API; the
important thing is the functionality.

I've been holding off responding to this patch until I get a chance to
ponder exactly what should be in 'blame -v' and how it might look, and
if there are any other extensions we want to consider in the future.
(For example, one realization is that when Subversion downloads the
raw blame information for rev R, it actually gets enough information
to do 'svn blame' for *all* "ChangedRevisions" <= R for that file.
This could be really interesting if you're looking for blame info on
content that you suspect might be deleted before R.  Showing all
ChangedRevs <= R might be a useful interpretation of 'blame -R'.)

Anyway, the question is complex enough that responding to the patch
immediately wasn't possible (for me), sorry about that.  Sander Roobol
won't let it slip away from the issue tracker, though.

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Add verbose support to blame

Posted by "C. Michael Pilato" <cm...@collab.net>.
Ben Collins-Sussman <su...@collab.net> writes:

> On Wed, 2004-02-18 at 15:24, C. Michael Pilato wrote:
> 
> > Hmm... perhaps I just remove the --quiet option I just added to
> > svndumpfilter and save that addition for the 1.1 branch.
> 
> It's already scheduled for 1.1, just by virtue of being in /trunk at the
> moment.  And I don't think you had any intention of backporting your
> svndumpfilter fix to 1.0.x, did you?

It crossed my mind, but now that I think about it -- why bother?

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Add verbose support to blame

Posted by Ben Collins-Sussman <su...@collab.net>.
On Wed, 2004-02-18 at 15:24, C. Michael Pilato wrote:

> Hmm... perhaps I just remove the --quiet option I just added to
> svndumpfilter and save that addition for the 1.1 branch.

It's already scheduled for 1.1, just by virtue of being in /trunk at the
moment.  And I don't think you had any intention of backporting your
svndumpfilter fix to 1.0.x, did you?



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Add verbose support to blame

Posted by "C. Michael Pilato" <cm...@collab.net>.
Mark Benedetto King <mb...@lowlatency.com> writes:

> On Wed, Feb 18, 2004 at 03:27:34PM -0500, Greg Hudson wrote:
> > On Wed, 2004-02-18 at 14:50, Ben Collins-Sussman wrote:
> > > I think 'svn blame -v' is definitely a cool new thing, but I'm -0.5 for
> > > nominating it for svn 1.0.1.
> > > 
> > > svn 1.0.1 is about fixing serious bugs... none of the bugs was serious
> > > enough to hold up the 1.0 release, but they're all still serious.
> > 
> > I agree.  1.0.x should not be adding any new features.  (We can fix
> > minor bugs as well as serious ones, as long as the fixes are simple, but
> > this is not a fix at all.)
> > 
> 
> It's not a big leap to say that a new feature is like a new function;
> downwards compatibility would be broken.

Hmm... perhaps I just remove the --quiet option I just added to
svndumpfilter and save that addition for the 1.1 branch.

I can see how command-line interfaces are similar to APIs, especially
when you think of scripts wrapping our binaries like functions calling
functions.  We shouldn't be changing our programs' option contracts or
their output.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Add verbose support to blame

Posted by Mark Benedetto King <mb...@lowlatency.com>.
On Wed, Feb 18, 2004 at 03:27:34PM -0500, Greg Hudson wrote:
> On Wed, 2004-02-18 at 14:50, Ben Collins-Sussman wrote:
> > I think 'svn blame -v' is definitely a cool new thing, but I'm -0.5 for
> > nominating it for svn 1.0.1.
> > 
> > svn 1.0.1 is about fixing serious bugs... none of the bugs was serious
> > enough to hold up the 1.0 release, but they're all still serious.
> 
> I agree.  1.0.x should not be adding any new features.  (We can fix
> minor bugs as well as serious ones, as long as the fixes are simple, but
> this is not a fix at all.)
> 

It's not a big leap to say that a new feature is like a new function;
downwards compatibility would be broken.

--ben


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Add verbose support to blame

Posted by Greg Hudson <gh...@MIT.EDU>.
On Wed, 2004-02-18 at 14:50, Ben Collins-Sussman wrote:
> I think 'svn blame -v' is definitely a cool new thing, but I'm -0.5 for
> nominating it for svn 1.0.1.
> 
> svn 1.0.1 is about fixing serious bugs... none of the bugs was serious
> enough to hold up the 1.0 release, but they're all still serious.

I agree.  1.0.x should not be adding any new features.  (We can fix
minor bugs as well as serious ones, as long as the fixes are simple, but
this is not a fix at all.)


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Add verbose support to blame

Posted by kf...@collab.net.
Ben Collins-Sussman <su...@collab.net> writes:
> I think 'svn blame -v' is definitely a cool new thing, but I'm -0.5 for
> nominating it for svn 1.0.1.
> 
> svn 1.0.1 is about fixing serious bugs... none of the bugs was serious
> enough to hold up the 1.0 release, but they're all still serious.

+1 on Ben's -0.5, and for the same reasons.

(I think I made comments earlier about the circumstances under which
this change could be *formally* permitted in 1.0.x; I was just
focusing too narrowly, sorry for misimplication.)

-Karl

<><><> Standard disclaimer:
<><><> 
<><><> If you detect inconsistencies between the positions espoused in
<><><> this post and positions espoused in previous posts, it's only
<><><> because you haven't been sitting under the same cosmic rays.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Add verbose support to blame

Posted by Ben Collins-Sussman <su...@collab.net>.
On Wed, 2004-02-18 at 13:46, Ben Reser wrote:

> > Committed with a tweak (moved the baton typedef into blame-cmd.c) in
> > revision 8678.
> 
> Great, I added it to the STATUS file for 1.0.1.

I think 'svn blame -v' is definitely a cool new thing, but I'm -0.5 for
nominating it for svn 1.0.1.

svn 1.0.1 is about fixing serious bugs... none of the bugs was serious
enough to hold up the 1.0 release, but they're all still serious.

When I first came up with the changeset 1.0.1 nominee list a couple of
weeks ago, I deliberately skipped over "nice enhancements" that happened
since 1.0 went into code-freeze.  And this change looks like a nice
enhancement to me, not a critical bug.  




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Add verbose support to blame

Posted by Ben Reser <be...@reser.org>.
On Wed, Feb 18, 2004 at 08:42:10AM -0500, Mark Benedetto King wrote:
> On Wed, Feb 18, 2004 at 02:04:08AM -0600, Ben Gollmer wrote:
> > On Tue, 17 Feb 2004 14:20:08 -0600, Ben Reser <be...@reser.org> wrote:
> > > FYI to everyone.  Ben Gollmer sent me an updated patch last night
> > > offlist.  I reviewed it and made some corrections.  I figure he'll be
> > > posting a new patch soon...
> > > 
> > > So if anyone is thinking about working on this don't.  It's done.
> > 
> > Yes, and here is the patch. Thanks to Ben Reser for judicious application
> > of the cluebat. Since no function signatures have changed, this should be applicable for 1.0.1.
> > 
> 
> Committed with a tweak (moved the baton typedef into blame-cmd.c) in
> revision 8678.

Great, I added it to the STATUS file for 1.0.1.

-- 
Ben Reser <be...@reser.org>
http://ben.reser.org

"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Add verbose support to blame

Posted by Mark Benedetto King <mb...@lowlatency.com>.
On Wed, Feb 18, 2004 at 02:04:08AM -0600, Ben Gollmer wrote:
> On Tue, 17 Feb 2004 14:20:08 -0600, Ben Reser <be...@reser.org> wrote:
> > FYI to everyone.  Ben Gollmer sent me an updated patch last night
> > offlist.  I reviewed it and made some corrections.  I figure he'll be
> > posting a new patch soon...
> > 
> > So if anyone is thinking about working on this don't.  It's done.
> 
> Yes, and here is the patch. Thanks to Ben Reser for judicious application
> of the cluebat. Since no function signatures have changed, this should be applicable for 1.0.1.
> 

Committed with a tweak (moved the baton typedef into blame-cmd.c) in
revision 8678.

--ben


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Add verbose support to blame

Posted by Ben Gollmer <be...@jatosoft.com>.
On Tue, 17 Feb 2004 14:20:08 -0600, Ben Reser <be...@reser.org> wrote:
> FYI to everyone.  Ben Gollmer sent me an updated patch last night
> offlist.  I reviewed it and made some corrections.  I figure he'll be
> posting a new patch soon...
> 
> So if anyone is thinking about working on this don't.  It's done.

Yes, and here is the patch. Thanks to Ben Reser for judicious application
of the cluebat. Since no function signatures have changed, this should be applicable for 1.0.1.

[[[
Add verbose option to blame.

The attack of the Bens!

Patch by: Ben Gollmer
Reviewed and minor fixes from: Ben Reser

* subversion/clients/cmdline/cl.h
  Add svn_cl_blame_baton_t struct

* subversion/clients/cmdline/blame-cmd.c
  (blame_receiver): Also output the date if verbose option is set.
  (svn_cl__blame): Use svn_cl_blame_baton_t struct for the baton, to
    pass the opt_state through to the receiver.

* subversion/clients/cmdline/main.c
  (svn_cl__cmd_table): Add a verbose option to blame.

]]]

Index: subversion/clients/cmdline/cl.h
===================================================================
--- subversion/clients/cmdline/cl.h	(revision 8661)
+++ subversion/clients/cmdline/cl.h	(working copy)
@@ -139,6 +139,13 @@
 } svn_cl__cmd_baton_t;
 
 
+typedef struct
+{
+  svn_cl__opt_state_t *opt_state;
+  svn_stream_t *out;
+} svn_cl__blame_baton_t;
+
+
 /* Declare all the command procedures */
 svn_opt_subcommand_t
   svn_cl__add,
Index: subversion/clients/cmdline/blame-cmd.c
===================================================================
--- subversion/clients/cmdline/blame-cmd.c	(revision 8661)
+++ subversion/clients/cmdline/blame-cmd.c	(working copy)
@@ -22,6 +22,7 @@
 #include "svn_error.h"
 #include "svn_pools.h"
 #include "svn_cmdline.h"
+#include "svn_time.h"
 #include "cl.h"
 
 
@@ -35,12 +36,30 @@
                 const char *line,
                 apr_pool_t *pool)
 {
-  svn_stream_t *out = baton;
+  svn_cl__opt_state_t *opt_state =
+    ((svn_cl__blame_baton_t *) baton)->opt_state;
+  svn_stream_t *out = ((svn_cl__blame_baton_t *)baton)->out;
+  apr_time_t atime;
+  const char *time_utf8;
+  const char *time_stdout;
   const char *rev_str = SVN_IS_VALID_REVNUM (revision) 
                         ? apr_psprintf (pool, "%6" SVN_REVNUM_T_FMT, revision)
                         : "     -";
-  return svn_stream_printf (out, pool, "%s %10s %s\n", rev_str, 
-                            author ? author : "         -", line);
+  
+  if (opt_state->verbose)
+    {
+      SVN_ERR (svn_time_from_cstring (&atime, date, pool));
+      time_utf8 = svn_time_to_human_cstring (atime, pool);
+      SVN_ERR (svn_cmdline_cstring_from_utf8 (&time_stdout, time_utf8, pool));
+      return svn_stream_printf (out, pool, "%s %10s %s %s\n", rev_str, 
+                                author ? author : "         -", 
+                                time_stdout , line);
+    }
+  else
+    {
+      return svn_stream_printf (out, pool, "%s %10s %s\n", rev_str, 
+                                author ? author : "         -", line);
+    }
 }
  
 
@@ -55,6 +74,7 @@
   apr_pool_t *subpool;
   apr_array_header_t *targets;
   svn_stream_t *out;
+  svn_cl__blame_baton_t bl;
   int i;
 
   SVN_ERR (svn_opt_args_to_target_array (&targets, os, 
@@ -89,7 +109,9 @@
     }
 
   SVN_ERR (svn_stream_for_stdout (&out, pool));
-
+  bl.opt_state = opt_state;
+  bl.out = out;
+  
   subpool = svn_pool_create (pool);
 
   for (i = 0; i < targets->nelts; i++)
@@ -101,7 +123,7 @@
                               &opt_state->start_revision,
                               &opt_state->end_revision,
                               blame_receiver,
-                              out,
+                              &bl,
                               ctx,
                               subpool);
       if (err)
Index: subversion/clients/cmdline/main.c
===================================================================
--- subversion/clients/cmdline/main.c	(revision 8661)
+++ subversion/clients/cmdline/main.c	(working copy)
@@ -161,7 +161,7 @@
     "Output the content of specified files or\n"
     "URLs with revision and author information in-line.\n"
     "usage: blame TARGET...\n",
-    {'r', SVN_CL__AUTH_OPTIONS, svn_cl__config_dir_opt} },
+    {'r', 'v', SVN_CL__AUTH_OPTIONS, svn_cl__config_dir_opt} },
 
   { "cat", svn_cl__cat, {0},
     "Output the content of specified files or URLs.\n"

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Add verbose support to blame

Posted by Ben Reser <be...@reser.org>.
On Mon, Feb 16, 2004 at 01:55:16AM -0600, Ben Gollmer wrote:
> You're right. When I was writing this patch, I was looking at
> log_message_receiver(), which takes a hash of changed paths. Upon
> closer examination, I see that changed_paths isn't really an output
> option; it's just additional information provided to the receiver.
> 
> I should have been looking at something like add-cmd.c, which pulls
> the options out of the baton.

FYI to everyone.  Ben Gollmer sent me an updated patch last night
offlist.  I reviewed it and made some corrections.  I figure he'll be
posting a new patch soon...

So if anyone is thinking about working on this don't.  It's done.

-- 
Ben Reser <be...@reser.org>
http://ben.reser.org

"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Add verbose support to blame

Posted by Ben Gollmer <be...@jatosoft.com>.
On Mon, 16 Feb 2004 00:31:20 -0600, Ben Reser <be...@reser.org> wrote:
> Actually on further thought this implementation is completely wrong.
> The blame receiver already gets the date.  Output options don't belong
> in our API, we're calling a receiver for the client to implement the
> output anyway it chooses.  

You're right. When I was writing this patch, I was looking at
log_message_receiver(), which takes a hash of changed paths. Upon closer
examination, I see that changed_paths isn't really an output option; it's just
additional information provided to the receiver.

I should have been looking at something like add-cmd.c, which pulls the options
out of the baton.

> b) One blame receiver and it uses the baton we provide to pass the
> verbose flag through.

This is what I was trying to accomplish with my patch.

-- 
Ben

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Add verbose support to blame

Posted by Ben Reser <be...@reser.org>.
On Sun, Feb 15, 2004 at 06:37:16PM -0800, Ben Reser wrote:
> On Sat, Feb 14, 2004 at 07:11:29PM -0600, Ben Gollmer wrote:
> > I was reading the thread 'annotate support for emacs', and I thought 
> > I'd give this patching thing a shot. Yes, the date format is verbose, 
> > but that's the way libsvn_subr/time.c said to do it.
> > 
> > Log:
> > * clients/cmdline/main.c: Add '-v | --verbose' switch support to
> >     svn blame.
> > * clients/cmdline/blame-cmd.c
> >     (svn__cl_blame): Pass the verbose argument to svn_client_blame().
> >     (blame_reciever): If the verbose argument is present, format the
> >     date and include it in the output.
> > * include/svn_client.h: Add 'svn_boolean_t verbose' argument to
> >     the declaration of svn_client_blame().
> > * libsvn_client/blame.c
> >     (svn_client_blame): Pass the verbose argument to blame receiver.
> 
> You changed the function signature.  So this isn't going to go into
> 1.0.x.  You'd have to make a svn_client_blame_verbose() to put it in
> 1.0.x.  

Actually on further thought this implementation is completely wrong.
The blame receiver already gets the date.  Output options don't belong
in our API, we're calling a receiver for the client to implement the
output anyway it chooses.  A client has two potential ways of handling
situations like this:

a) Two blame receiver implementations.  One for verbose and non-verbose
output.  It just passes a different receiver to blame based on which one
it wnats.

b) One blame receiver and it uses the baton we provide to pass the
verbose flag through.

The later of which is IMHO the better option in this case since the
implementation difference is tiny.  Neither of these changes require any
change in our public API, they only change implementation details of our
client.  Then the only questions are:

1) Is it okay to add an enhancement in a patch release.  Which I think
the answer to depends on the enhancement.

2) Is it okay to add a command line option in a patch release?  This is
stickier.  Do we want to offer the same compatability guarantees to
scripts (forwards and backwards between patch releases)?  I don't think
this is necessary, scripts can more easily be written to detect the
functionality available and to use what they can or can't.  

I'm inclined to say that both of these would be okay in this
circumstance.  The change would be relatively minor from the end user
standpoint, makes it possible to make the annotation feature in emacs
work better/right.   

-- 
Ben Reser <be...@reser.org>
http://ben.reser.org

"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Release Timing (was [PATCH] Add verbose support to blame)

Posted by Ben Reser <be...@reser.org>.
On Sun, Feb 15, 2004 at 11:19:03PM -0500, Greg Hudson wrote:
> Nope.  1.0.1 needs to be both forward- and backward-compatible with
> 1.0.0, so we can't add new functions.
> 
> (Generally speaking, 1.0.x should only be fixing bugs in 1.0.0, and then
> only bugs which can be fixed simply--crashes, typos, etc.  Things like
> "make diff work across renames" should wait for 1.1.x, since they will
> involve substantial amounts of code.)

That means by my best guess we're talking 2-3 months for a fix in our
core functionality (and I'm talking about diffs across renames).
Perhaps we need to reconsider the soak time policy for releases.  

4 weeks for major releases is probably okay.  We won't do them all that
often and it won't hurt us to spend 4 weeks on tools, documentation,
etc..  Plus it gives us time to work with authors of programs that have
dependencies to help them get ready for the changes that go along with a
major release.

But for minor releases I think it's excessive.  I'd at least half the
soak time for them.  We can't introduce anything that would break API
compatibility so we don't need the time to work with the other projects.
Documentations, tools, etc... will have a much easier time keeping up
with changes since they will be more gradual.

Dropping the soak time for minor releases to 2 weeks would put us at a
1.1 as early as the first part of April.  About a month after 1.0.  

I think it's obvious that we have issues that we can't fix without some
major code changes.  The sooner we get those things fixed and in a
release the fewer people that will encounter them and the easier it will
be to justify uptake of our project.

Maybe down the road where the changes we're thinking need done are
mostly enhancements, not bug fixes that just don't fit into the
compatibility requirements of a patch release, we can increase the soak
time and slow back down.  But at this point I think we've slowed
the releases too much.

(Yes I don't consider -v on blame a bug fix...  In fact after thinking
about it some more, it doesn't need an API change at all to get it in
1.0.1.  But ghudson's response did make me start thinking about and
looking at the scheduling down the line and realizing it seemed bad to
me).

-- 
Ben Reser <be...@reser.org>
http://ben.reser.org

"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Add verbose support to blame

Posted by Greg Hudson <gh...@MIT.EDU>.
On Sun, 2004-02-15 at 23:07, Ben Reser wrote:
> I'll rework the patch to have a svn_client_blame_verbose()...

> Might as well get it into 1.0.1.

Nope.  1.0.1 needs to be both forward- and backward-compatible with
1.0.0, so we can't add new functions.

(Generally speaking, 1.0.x should only be fixing bugs in 1.0.0, and then
only bugs which can be fixed simply--crashes, typos, etc.  Things like
"make diff work across renames" should wait for 1.1.x, since they will
involve substantial amounts of code.)


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Add verbose support to blame

Posted by Ben Reser <be...@reser.org>.
On Sun, Feb 15, 2004 at 09:49:41PM -0600, Ben Gollmer wrote:
> Yeah, I know... I just wanted to get familiar with some subversion code, and
> this task looked about as bite-sized as it was gonna get :)
> 
> If the patch looks reasonable, it can be filed away and used later (1.x.x
> series). If not, maybe someone (the emacs guys) will find it useful as a local
> patch for now.

Nope it can't be used until 2.0.0.  Function signatures can't change
except between majors.  I'll rework the patch to have a
svn_client_blame_verbose()...

Might as well get it into 1.0.1.  When we get around to 2.0 we'll need
to do some cleanup on the API anyway.

-- 
Ben Reser <be...@reser.org>
http://ben.reser.org

"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Add verbose support to blame

Posted by Ben Gollmer <be...@jatosoft.com>.
On Sun, 15 Feb 2004 20:37:16 -0600, Ben Reser <be...@reser.org> wrote:
> On Sat, Feb 14, 2004 at 07:11:29PM -0600, Ben Gollmer wrote:
> > I was reading the thread 'annotate support for emacs', and I thought 
> > I'd give this patching thing a shot. Yes, the date format is verbose, 
> > but that's the way libsvn_subr/time.c said to do it.
>
> You changed the function signature.  So this isn't going to go into
> 1.0.x.  You'd have to make a svn_client_blame_verbose() to put it in
> 1.0.x.  

Yeah, I know... I just wanted to get familiar with some subversion code, and
this task looked about as bite-sized as it was gonna get :)

If the patch looks reasonable, it can be filed away and used later (1.x.x
series). If not, maybe someone (the emacs guys) will find it useful as a local
patch for now.

-- 
Ben

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Add verbose support to blame

Posted by Ben Reser <be...@reser.org>.
On Sat, Feb 14, 2004 at 07:11:29PM -0600, Ben Gollmer wrote:
> I was reading the thread 'annotate support for emacs', and I thought 
> I'd give this patching thing a shot. Yes, the date format is verbose, 
> but that's the way libsvn_subr/time.c said to do it.
> 
> Log:
> * clients/cmdline/main.c: Add '-v | --verbose' switch support to
>     svn blame.
> * clients/cmdline/blame-cmd.c
>     (svn__cl_blame): Pass the verbose argument to svn_client_blame().
>     (blame_reciever): If the verbose argument is present, format the
>     date and include it in the output.
> * include/svn_client.h: Add 'svn_boolean_t verbose' argument to
>     the declaration of svn_client_blame().
> * libsvn_client/blame.c
>     (svn_client_blame): Pass the verbose argument to blame receiver.

You changed the function signature.  So this isn't going to go into
1.0.x.  You'd have to make a svn_client_blame_verbose() to put it in
1.0.x.  

-- 
Ben Reser <be...@reser.org>
http://ben.reser.org

"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org