Re: svn commit: r14921 - in trunk: subversion/clients/cmdline subversion/clients/cmdline/dtd tools/client-side

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r14921 - in trunk: subversion/clients/cmdline subversion/clients/cmdline/dtd tools/client-side

Philip Martin
[hidden email] writes:

> Author: lundblad
> Date: Wed Jun  1 15:47:41 2005
> New Revision: 14921
>
> Added:
>    trunk/subversion/clients/cmdline/dtd/status.dtd
> Modified:
>    trunk/subversion/clients/cmdline/cl.h
>    trunk/subversion/clients/cmdline/main.c
>    trunk/subversion/clients/cmdline/status-cmd.c
>    trunk/subversion/clients/cmdline/status.c
>    trunk/subversion/tests/clients/cmdline/stat_tests.py
>    trunk/tools/client-side/bash_completion
>
> Log:
> Fix issue 2069 - "svn status" in xml mode.

I'm not sure who or what is going to use xml mode, but it shows less
information that plain status.  The issue cites long user names
containing spaces but the xml doesn't appear to include user names?
The xml doesn't show revision numbers either, so it's impossible to
distinguish single and mixed revision working copies.  Is this just
XML for the sake of it?  Perhaps we should just have a non-xml mode
that drops the username instead?

--
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r14921 - in trunk: subversion/clients/cmdline subversion/clients/cmdline/dtd tools/client-side

Daniel Rall
On Wed, 2005-06-01 at 23:25 +0100, Philip Martin wrote:

> [hidden email] writes:
>
> > Author: lundblad
> > Date: Wed Jun  1 15:47:41 2005
> > New Revision: 14921
> >
> > Added:
> >    trunk/subversion/clients/cmdline/dtd/status.dtd
> > Modified:
> >    trunk/subversion/clients/cmdline/cl.h
> >    trunk/subversion/clients/cmdline/main.c
> >    trunk/subversion/clients/cmdline/status-cmd.c
> >    trunk/subversion/clients/cmdline/status.c
> >    trunk/subversion/tests/clients/cmdline/stat_tests.py
> >    trunk/tools/client-side/bash_completion
> >
> > Log:
> > Fix issue 2069 - "svn status" in xml mode.
>
> I'm not sure who or what is going to use xml mode, but it shows less
> information that plain status.  The issue cites long user names
> containing spaces but the xml doesn't appear to include user names?
> The xml doesn't show revision numbers either, so it's impossible to
> distinguish single and mixed revision working copies.  Is this just
> XML for the sake of it?  Perhaps we should just have a non-xml mode
> that drops the username instead?

I'm not one to use XML for XML's sake, but I would like to note that
CruiseControl use the XML mode of 'svn log'.



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r14921 - in trunk: subversion/clients/cmdline subversion/clients/cmdline/dtd tools/client-side

Peter N. Lundblad
In reply to this post by Philip Martin
On Wed, 1 Jun 2005, Philip Martin wrote:

> [hidden email] writes:
>
> > Author: lundblad
> > Date: Wed Jun  1 15:47:41 2005
> > New Revision: 14921
> >
> > Added:
> >    trunk/subversion/clients/cmdline/dtd/status.dtd
> > Modified:
> >    trunk/subversion/clients/cmdline/cl.h
> >    trunk/subversion/clients/cmdline/main.c
> >    trunk/subversion/clients/cmdline/status-cmd.c
> >    trunk/subversion/clients/cmdline/status.c
> >    trunk/subversion/tests/clients/cmdline/stat_tests.py
> >    trunk/tools/client-side/bash_completion
> >
> > Log:
> > Fix issue 2069 - "svn status" in xml mode.
>
> I'm not sure who or what is going to use xml mode, but it shows less
> information that plain status.  The issue cites long user names
> containing spaces but the xml doesn't appear to include user names?
> The xml doesn't show revision numbers either, so it's impossible to
> distinguish single and mixed revision working copies.  Is this just

both showing last committed info and WC revisions are possible
improvements. Alexander: want to work on these?

OTOH, if you actually read the DTD, you'll see that the XML mode shows
more information regarding out-of-date status and WC and repository locks.

> XML for the sake of it?  Perhaps we should just have a non-xml mode
> that drops the username instead?
>
I don't know where you got that impression. Just because there is one
field missing that you want doesn't make it useless, does it? And leaving
out -v already avoids printing usernames. But if you *need* those
usernames that's not an option (or last committed date or something).

But what this and other patches adding --xml output wants to solve is that
we can't guarantee that our output will stay the same forever.  We had to
make an incompatible change to the svn update output in 1.3, for example.
Fortunately, status had unused columns available this time, but that may
obviously not be the case in the future.

Also, this issue has been around for some time and the patch have
circulated six times on the mailing list.  If you have objections to the
functionality - adding xml output to status - IMO it makes much more sense
to raise those objections earlier in the process than after the commit and
I'm actually getting a little annoyed by it.  I'm not saying it's too late
to revert this work and if others dislike it, that will be done of course.
But it's not like this feature has been implemented invissibly in any way.

Regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r14921 - in trunk: subversion/clients/cmdline subversion/clients/cmdline/dtd tools/client-side

kfogel
"Peter N. Lundblad" <[hidden email]> writes:
> Also, this issue has been around for some time and the patch have
> circulated six times on the mailing list.  If you have objections to the
> functionality - adding xml output to status - IMO it makes much more sense
> to raise those objections earlier in the process than after the commit and
> I'm actually getting a little annoyed by it.  I'm not saying it's too late
> to revert this work and if others dislike it, that will be done of course.
> But it's not like this feature has been implemented invissibly in any way.

Hey Peter,

I agree with everything you said about it how this code already serves
a useful purpose even without further enhancements, and how it's easy
to improve on it now that it's in Subversion.  However, I think in the
above paragraph you might be asking for something unrealistic.

Sure, ideally everyone would make their comments before a patch goes
in, especially when it's been circulating as long as this one has.

But in fact, no one has time to watch every iteration of every patch.
To deal with this, we have (unconsciously) adopted a pretty effective
divide-and-conquer approach: for each patch undergoing review, there
are one or two people paying close attention and giving detailed
feedback.  When the patch is committed, it has passed that first, most
important level of filtering, so now other people start to look more
closely at it, offering comments and suggestions.  They invest the
time now because it has become clear that the investment is worth it.
The patch has already been cleaned up of obvious bugs and problems,
and it's in the core code.

Don't think of the commit as a static, final event in the lifecycle of
the patch.  Think of it as another turn of the review loop -- but a
special turn, one that serves as a signal to those who have been
sitting on the sidelines: "Now is your moment, go take a closer look,
because the best time to fix any remaining problems is when the code
is fresh and hasn't yet grown dependencies."

So please don't be annoyed.  You did a great service by paying close
attention to this patch from the beginning, but that doesn't mean it's
wrong for others to start paying attention now.

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r14921 - in trunk: subversion/clients/cmdline subversion/clients/cmdline/dtd tools/client-side

Philip Martin
[hidden email] writes:

> So please don't be annoyed.  You did a great service by paying close
> attention to this patch from the beginning, but that doesn't mean it's
> wrong for others to start paying attention now.

I didn't look at the patches because I have no real interest in XML
and I'm lazy.  When it appeared on the commit list I tried it to see
what it did, and I was surprised at how little information was shown.
So I looked at the issue and was further surprised that the user name
problem cited there wasn't solved.  If the XML output is useful to
someone then I'm happy for it to exist, but I do wonder what sort of
program is going to use it.

--
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]