Re: Improving svn commit progress notification

Ivan Zhakov wrote:
>> [[[
>> $ svn ci wc -m "log msg"
>> Sending        wc\foo
>> Transmitting file data ............done
>> Committing transaction...
>> Committed revision 5.
>> ]]]
>>
>> Also consider the out-of-date case:
>
> Committed in r1603388 and r1604179. I'm going to implement showing
> percentage of deltas transmitted so far later.

I like this, basically.

A few small concerns...

Index: subversion/include/svn_wc.h
+  /** Finalizing commit.
+   * @since New in 1.9. */
+  svn_wc_notify_commit_finalizing
 } svn_wc_notify_action_t;

This is a public API. We should document what other fields the notification receiver will receive: in this case, it's the "url" field.

I know we haven't done that in other cases, but we should.

Index: subversion/svn/notify.c
+       case svn_wc_notify_commit_finalizing:
+      if (nb->sent_first_txdelta)
+        {
+          SVN_ERR(svn_cmdline_printf(pool, _("done\n")));

I understand we don't want to print "done" if we did not print "Sending.......",

+          SVN_ERR(svn_cmdline_printf(pool, _("Committing transaction...")));

but I think it is inconsistent to not print "Committing transaction..." for a commit that only performed tree changes. Part of your rationale for the change was to let the user know that all of the relevant changes have been transmitted and now we are waiting for the server to finalize the txn. This rationale applies equally well for a commit that did not include text deltas.

Certain operations such as "svn propset p v URL" can only perform one change (one prop change in this case). With a one-change operation, we could consider that this extra line is ugly -- it's just noise really -- one line of notification is enough. If that was your reasoning, we should implement that.

+      }

(nit: Wonky indentation on that closing brace.)

- Julian
Received on 2014-06-20 18:41:59 CEST