Re: Reviewing merges

2010-05-01 Thread Gilles Moris
On Friday 30 April 2010 10:52:53 pm Andrew wrote:
 Hi all.  Is there any support for reviewing merges (changesets with
 two parents)?  I imagine that ideally, it would be a review request
 that has two diffs -- one against the first parent and the other
 against the second parent.

I guess you're still talking about Mercurial.
In Mercurial, the diffs are always done conventionally against the first 
parent, including merges. This is in particular the case with the hg diff -c 
cset command. This makes sense are you are usually pulling some branch 
changes, and this is what you want to review and what you specify to the hg 
merge command.

When you use the mercurial-reviewboard extension, you can still use 
the --parent option to specify the parent from which computing the diff.

Regards.
Gilles.

-- 
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


Re: How apply patches in pre-review process?

2010-05-01 Thread Jan Koprowski


On Apr 30, 7:58 pm, Chris Clark chris.cl...@ingres.com wrote:
 Jan Koprowski wrote:
  . When review is done and submitted
  developer can change everything he want in working tree and commit
  everything he want. I'm thinking how be sure that reviewed changes is
  commited changes.

 For us, we are just using the honour/honor system, i.e. no checks :-)
 This mostly works BUT occasionally people do make mistakes (e.g. submit
 an extra file that was not in the review) so a check would be nice, for
 me personally I'm not sure it is worth spending the time on it.

Implementation of this is time consuming. This problem disappear  in
post-review process (at least in my model). In my MA I assume I don't
trust developer in anything.

 The other idea similar to your manual reviewer idea would be for a
 script to pull the diff from RB (that is marked with Ship It!) and
 submit the change (assuming the patch applies cleanly). If patches don't
 apply cleanly the review could be marked with patch didn't apply
 cleanly to [rev in SCM] and the developer an resubmit. If you already
 have a buildbot system this is probably your best option (especially if
 you have tests as part of the build bot, you could add additional
 rejection checks).

I use CruiseControl and Hudson in my system. I don't see simple way to
apply patches cleanly. This is impossible. The problem is from which
point of development tree You create branch. When You create 10
branches, then locally make changes for each of them and submit one by
another to review. Look what will happen.
Central SVN repo doesn't know about Your changes because there are
only on Your computer. So You submit first review, merge to trunk and
ok. You get second review, try to merge to trunk but - You merge this
second review with code different then their baseline (to different
code then this from You branch). This is the nest of potential
problems. I don't imagine this process without human intervention.

 I've deliberately minimized the procedures/policies we have for RB at
 our site so this isn't something I plan on implementing myself but I can
 see the value.

IMHO much more simple is post-review. All developer commit to one
development branch but with each branch there is related ticket in
issue tracker with changes when I have list of revision numbers made
for this particular change. In hooks and in build machine I can still
check status of this ticket and I know what is the status of code
related with them. Also I can controll access to repository based on
status.
The great profit is the developers for all the time integrate the code
in the branch by him self. This is grate information. But there is two
problems:


1) ReviewBoard doesn't like diffs from not continuous revisions like:
1-3, 8-9. This is not a big problem I think but no one try this
before. Opportunity is allow save in ticket only continuous the
highest. But if we have two tickets:
a) revision 1-9
b) revision 3-7
First one will include changes related with second. This is a little
problem.

2) Second problem is related with first. Before build we must check
status of all tickets which was reviewed and clever find place until
all code was reviewed. This is not very hard but need to be made. We
also need to know where last build end to know which revision number
is our baseline :D

But I prefer this model then pre-review :D

-- 
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