Actually, I wouldn't mind such a parameter. (though, --diff-filename, as we
don't use underscores in parameter names). There's actually a patch up for
review for accepting via stdin (though it's a bit stale and isn't the design
we want), so I guess there are more people who want this.

If you'd like to contribute it, we'd take it, but I'd ask that you make the
diff uploading part generic and not part of any SCMClient subclass, and that
it allow "-" as a value, at which point we'd read from stdin. We should be
able to just bypass the diff generation part of the SCMClient and instead
read the data in ourselves and use it.

Christian

-- 
Christian Hammond - chip...@chipx86.com
Review Board - http://www.reviewboard.org
VMware, Inc. - http://www.vmware.com


On Thu, Nov 5, 2009 at 11:38 AM, Chris Clark <chris.cl...@ingres.com> wrote:

>
> Background:
>
>    We have some VMS users using postreview, observant readers may
>    recall that process spawning under VMS is a nightmare to the point
>    that neither CPython or the JVM have a nice portable way of doing
>    this. This causes postreview no end of trouble under VMS whether one
>    uses CPython or Jython.
>
>
> We added a "take this diff file and post to the server" flag, extract
> from postreview -h:
>
> --diff_filename=DIFF_FILENAME
>                      INTERNAL ONLY: file containing diffs/change, i.e. do
>                      not perform diff, just post provided file
>
> This may be one option for dealing with this issue where you need to "do
> custom stuff with the underling scm/diff" , the work flow then becomes:
>
>    svn (or what ever) ........ > mydiff.diff
>    postreview ....... --diff_filename mydiff.diff .....
>
>
> Now, I hooked this into the SCM specific subclass of SCMClient() as I
> didn't anticipate anyone else wanting this. You might want to give that
> a try.
>
> Chris
>
>
>
>
> Nathan Heijermans wrote:
> > I've had a similar problem in the past that I currently work around by
> > manually generating a fake diff file. I encounter the problem when I'm
> > trying to review a directory from Subversion that has not been reviewed
> > yet. Ideally, I would like to be able to do `svn diff -r X:HEAD` to
> > generate the diff, where X is the revision prior to the creation of the
> > particular directory I want to review. svn diff doesn't like that
> > though, because the directory didn't exist at revision X; it was created
> > in X+1. I don't know of a way to work around that with the `svn`
> > command-line tool.
> >
> > Nathan Heijermans
> >
> > Christian Hammond wrote:
> >
> >> svn diff -r PREV doesn't do what you want as far as generating the diff?
> >>
> >> I don't like the idea of faking the diff using svn cat. It's a new
> >> code path to maintain that doesn't work entirely like svn diff, and
> >> means we have to now fake a diff. Personally I'd say that if we can't
> >> make it work with svn diff's parameters, we shouldn't do it in
> >> post-review. But we should be able to make it work.
> >>
> >> Christian
> >>
> >> --
> >> Christian Hammond - chip...@chipx86.com <mailto:chip...@chipx86.com>
> >> Review Board - http://www.reviewboard.org
> >> VMware, Inc. - http://www.vmware.com
> >>
> >>
> >> On Tue, Nov 3, 2009 at 10:58 AM, Pv <p...@swooby.com
> >> <mailto:p...@swooby.com>> wrote:
> >>
> >>
> >>     Yes, I want to do a post-commit review of the new files
> added/created
> >>     in an SVN revision without making any actual changes to the already
> >>     checked in files.
> >>
> >>     The diff is indeed empty (I already knew this, just thought it was
> >>     obvious and didn't share).
> >>
> >>     Basically, two possibilities:
> >>     1) How to get "svn diff" to output a diff of the HEAD.
> >>      I played w/ "svn diff -r HEAD", and same w/ COMMIT, etc, but
> >>     couldn't get it to output anything.
> >>      I must just be missing a switch.
> >>     2) post-review could detect (or have a switch) that if only one
> >>     revision # is specified (ex: --revision-range=1234) and the diff is
> >>     empty, then try something like an "svn cat", but with a diff
> >>     compatible output.
> >>
> >>     Pv
> >>
> >>     On Nov 2, 12:41 pm, Christian Hammond <chip...@chipx86.com
> >>     <mailto:chip...@chipx86.com>> wrote:
> >>     > It sounds like the generated diff is empty. If you add
> >>     --output-diff to
> >>     > those parameters, you should be able to see for sure.
> >>     >
> >>     > if you don't specify a to:from, it will do, in your case,
> >>     24506:HEAD. If
> >>     > that's what you're wanting, then we should find out why exactly
> >>     this is
> >>     > failing, but if that's not the expected range, you'll need to
> >>     specify your
> >>     > own from:to.
> >>     >
> >>     > I don't entirely understand what your scenario is. Are you
> >>     saying that there
> >>     > are new files that were created in r24506 and those are the ones
> >>     you want to
> >>     > put up for review?
> >>     >
> >>     > Christian
> >>     >
> >>     > --
> >>     > Christian Hammond - chip...@chipx86.com <mailto:
> chip...@chipx86.com>
> >>     > Review Board -http://www.reviewboard.org
> >>     > VMware, Inc. -http://www.vmware.com
> >>     >
> >>     >
> >>     >
> >>     > On Thu, Oct 29, 2009 at 12:07 PM, Pv <p...@swooby.com
> >>     <mailto:p...@swooby.com>> wrote:
> >>     >
> >>     > > I am using the following command to do a post-commit review on
> >>     a first
> >>     > > time checkin:
> >>     > > post-review -d --revision-range=24506
> >>     >
> >>     > > Obviously, I am not specifying a from:to range, but since this
> >>     is a
> >>     > > first time checking any earlier "from" svn rev # wouldn't
> >>     exist in the
> >>     > > newly created path.
> >>     >
> >>     > > The [debug] output I get is this:
> >>     > > >>> svn info
> >>     > > >>> repository info: Path: svn://devsrvsea01/svnroot, Base path:
> >>     > > /trunk/StopProcess, Supports changesets: False
> >>     > > >>> svn propget reviewboard:url C:\svns\tps\trunk\StopProcess
> >>     > > >>> svn propget reviewboard:url C:\svns\tps\trunk\
> >>     > > >>> svn diff --diff-cmd=diff -r 24506
> >>     > > >>> Looking for '10.100.120.86 /' cookie in C:\Documents and
> >>     > > Settings\User\Local Settings\Application
> >>     Data\.post-review-cookies.txt
> >>     > > >>> Loaded valid cookie -- no login required
> >>     > > >>> Attempting to create review request for None
> >>     > > >>> HTTP GETting /api/json/repositories/
> >>     > > >>> HTTP GETting /api/json/repositories/1/info/
> >>     > > >>> repository info: Path: svn://devsrvsea01/svnroot, Base path:
> >>     > > /trunk/TPS/QA/StopProcess, Supports changesets: False
> >>     > > >>> HTTP POSTing
> >>     tohttp://10.100.120.86/api/json/reviewrequests/new/
> >>     <http://10.100.120.86/api/json/reviewrequests/new/>:
> >>     > > {'repository_path': 'svn://devsrvsea01/svnroot'}
> >>     > > >>> Review request created
> >>     > > >>> Uploading diff, size: 0
> >>     > > >>> HTTP POSTing to
> >>     >
> >>     >
> http://10.100.120.86/api/json/reviewrequests/14/diff/new/:{'basedir<http://10.100.120.86/api/json/reviewrequests/14/diff/new/:%7B%27basedir>
> >>     <
> http://10.100.120.86/api/json/reviewrequests/14/diff/new/:%7B%27basedir>':
> >>     > > '/trunk/TPS/QA/StopProcess'}
> >>     > > Error uploading diff: One or more fields had errors (105)
> >>     > > >>> {'fields': {'path': ['The submitted file is empty.']},
> >>     'stat': 'fail',
> >>     > > 'err': {'msg': 'One or more fields had errors', 'code': 105}}
> >>     > > Your review request still exists, but the diff is not attached.
> >>     >
> >>     > > Since
> >>     > > > To unsubscribe from this group, send email to
> >>     > > 
> >> reviewboard+unsubscr...@googlegroups.com<reviewboard%2bunsubscr...@googlegroups.com>
> >>     
> >> <mailto:reviewboard%2bunsubscr...@googlegroups.com<reviewboard%252bunsubscr...@googlegroups.com>
> ><reviewboard%2bunsubscr...@googlegr
> >>     oups.com <http://oups.com>>
> >>     > > For more options, visit this group at
> >>     > >http://groups.google.com/group/reviewboard?hl=en
> >>     > > -~----------~----~----~----~------~----~------~--~---
> >>     To unsubscribe from this group, send email to
> >>     
> >> reviewboard+unsubscr...@googlegroups.com<reviewboard%2bunsubscr...@googlegroups.com>
> >>     
> >> <mailto:reviewboard%2bunsubscr...@googlegroups.com<reviewboard%252bunsubscr...@googlegroups.com>
> >
> >>     For more options, visit this group at
> >>     http://groups.google.com/group/reviewboard?hl=en
> >>     -~----------~----~----~----~------~----~------~--~---
> >>
> >>
> >>
> >>
> >>> To unsubscribe from this group, send email to
> >>>
> >> reviewboard+unsubscr...@googlegroups.com<reviewboard%2bunsubscr...@googlegroups.com>
> >> For more options, visit this group at
> >> http://groups.google.com/group/reviewboard?hl=en
> >> -~----------~----~----~----~------~----~------~--~---
> >>
> >>
> >
> >
> > > To unsubscribe from this group, send email to
> reviewboard+unsubscr...@googlegroups.com<reviewboard%2bunsubscr...@googlegroups.com>
> > For more options, visit this group at
> http://groups.google.com/group/reviewboard?hl=en
> > -~----------~----~----~----~------~----~------~--~---
> >
> >
>
>
> > To unsubscribe from this group, send email to
> reviewboard+unsubscr...@googlegroups.com<reviewboard%2bunsubscr...@googlegroups.com>
> For more options, visit this group at
> http://groups.google.com/group/reviewboard?hl=en
> -~----------~----~----~----~------~----~------~--~---
>
>

--~--~---------~--~----~------------~-------~--~----~
Want to help the Review Board project? Donate today at 
http://www.reviewboard.org/donate/
Happy user? Let us know at http://www.reviewboard.org/users/
-~----------~----~----~----~------~----~------~--~---
To unsubscribe from this group, send email to 
reviewboard+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/reviewboard?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to