On Thu, Jan 08, 2004 at 02:33:22PM -0600, kfogel@collab.net wrote:
> (Poor MBK, it seems he's always producing patches based on discussion,
> and then people are realizing concerns afterwards and vetoing them!)
I don't mind; I feel that strawman patches are better than no patches,
especially in that they help to ensure that progress is made, however
much discussion it takes. Otherwise, we risk talking in circles.
>
> I've vetoed issue #1682 for 1.0, at least while we discuss. I don't
> think it's compelling for 1.0. Context:
>
> * Issue #1682
> 'svn blame' should adjust to max username width
> Justification: Improved blame display, API change, low risk.
> Notes: bindings would need updating
> Votes:
> +1: mbk
> -1: kfogel (see mailing list thread foo, meaning, I'll put the
> URL and Message-ID here as soon as I've got them. :-) )
>
> Here are the reasons I'm against it (and MBK, sorry I didn't manage to
> crystallize these concerns earlier):
>
> We don't absolutely *need* this enhancement right now. 'svn blame' is
> still very useful without this change; this is an edge-case thing.
>
You're right.
> Furthermore, the patch is not really trivial. In the issue, MBK calls
> the patch "simple", but it doesn't look very simple to me. It's a
> medium-sized code change -- admittedly, it's partly rearrangement, but
> it's also an API change. And there would need to be another bindings
> change to compensate, which we don't have a patch for yet.
It's simple in that the change is shallow, and no rocket science or
subtlety is involved. I renamed the old typedef to make the difference
between it and the new one clearer, otherwise the patch would have been
smaller.
>
> For me, this issue is kind of the deciding moment in the API stability
> question.
>
> If you believe (as I do) that we are *bound* to discover more API
> issues after we release 1.0 anyway, and that they will necessitate an
> API-changing release sooner rather than later anyway, then this patch
> should be left out of 1.0.
>
> On the other hand, if you think that we've got all the API issues
> nailed down as part of this stabilization process, then including this
> patch in 1.0 makes sense. I think that would be waaaay optimistic,
> though. After all, we discovered this API change because of a
> feature/enhancement we wanted to implement. "Want to adjust blame
> output to usernames? Ah, just need this little API tweak..."
The svn_client_blame interface is much newer than, say, the svn_client_diff
interface. It's only natural that it see some feature-creep. I believe
the new interface to be significantly better in that essentially all blame
metadata is transmitted to the caller; any future features based on the
computed blame can be implemented in svn_cl__blame().
>
> But surely, we're going to discover even more things like that *after*
> we release 1.0, when the number of users and bindings-users increases
> dramatically. Some of these future discoveries will be more serious
> than this issue, too.
>
Surely we will discover more of these things, but several people seem to
have done a reasonably thorough review of our public APIs with an eye
towards future requirements.
> If we keep enslaving ourselves to the most recent API concern we
> happen to have thought of, how will we ever reach 1.0? We need to
> draw a line; although there is some fuzziness, I think this
> enhancement comes down on the other side of that ilne.
>
1.0 could certainly endure a "death of 1000 cuts" if we allow it to.
> Oh, and indepently of the API question:
>
> This is also a UI change. Here is an IRC conversation ghudson and I
> had, regarding the hastiness with which we settled this new UI:
>
> ghudson: I'm not bullish on #1682, because I'm not sure it's
> consistent with what we do elsewhere, and it seems a bit late
> to be making a snap UI decision like that. Also, it doesn't
> strike me as the cleanest API.
>
> ghudson: (Like, it feels like an API designed to allow exactly one
> kind of presentation, which we may or may not settle on,
> rather than something generally useful.)
>
I don't like to disagree with Greg Hudson without careful consideration,
but in this case I do: the new callback exposes all of the computed blame
information; svn_client_blame() has nothing left to give!
> kfogel: ghudson: exactly. this probably needs more discussion. And,
> I *do* think its okay to change the blame UI later. Some
> people will write parsers for fixed-width blame output, and
> they'll get burned, but we can document that people shouldn't
> count on it, and that will save at least some of them from
> future pain.
>
> In other words, this whole enhancement (both UI and API) needs more
> discussion. We should give ourselves enough leisure to have that
> discussion -- and that means not trying to squeeze this into 1.0.
>
Fine; I've taxed the code-review process with down-to-the-wire changes
enough for one release already (perhaps two! :-).
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jan 9 01:24:24 2004