Hi, Thanks for the comments, I just sent a fairly small patch for review to the list. I would like to see even changes like this reviewed, even if most if the time the reply is just LGTM.
On Tue, Mar 15, 2011 at 9:44 AM, Dalai Felinto <dfeli...@gmail.com> wrote: > Specific things would be nice to have: > =========================== > 1) raw diff between patches - this is not a big of a deal. But > somethings it's handy to quickly look at a raw diff file to see what > changed between patches. The tool does support side-by-side views of > patches with highlighting of changes, so it's not a big deal for small > patches. I think stepping through the files that changed between two patches is pretty quick, with j or J key, and it does show you which files changed on the front page, it's just that you happened to change all of them in each patch I think. > 2) upload files - a good patch comes with a good army of test files. > Of course one can upload a file somewhere else (I used the original > tracker ;). But Right, this is lacking. We could mail the list with attachments, but that's not ideal either. > 3) merge system - this is probably only a bug, but: if any of the > files you are patching changed after you created the file the upload > may fail (it failed in my first try) with no warning ! (so it indeed > created a new 'ticket'/entry but the diffs were all invalid). Perhaps we should report the lack of warning for this as a bug. Still need to make a wiki page, so will note it there too. > 4) more space to comment when you add a new patch - not a big deal > since you can add a limitless comment when publishing the > comment-snippets, but I was surprised to be constrained by a > twitter-like limit. You can make the text area bigger, either dragging or using the + button depending on the browser. > 5) merge of comments: > A new system will require a new dynamic. That goes without saying. > Trying a "workflow" on the spot I noticed that (for the current > system) it may be interesting to merge comments. Otherwise a system of > round of review may need to be respected. And this can harm one of the > nicest things about this system - how anyone can review the patch at > any time. > > For the current system, if one sends a new patch too early (e.g. that > doesn't address all the comments in the 1st patch and still need > review from new devs) some information can get lost. For example, (1) > I sent a patch (2) inline commented my own code (3) people > review/comment some points (4) I send a new patch (5) inline commented > the changes in new patch (6) a new reviewer comes in - s/he will not > be able to read the comments in (2) even if that part of the code is > still present in the new patch (s/he can read it, but only if s/he > goes through the individual batch of comments or patches). Fair enough, I think it's good to skim through the existing comments before reviewing anyway, but it would be good if they got preserved between patches in the code. Brecht. _______________________________________________ Bf-committers mailing list Bf-committers@blender.org http://lists.blender.org/mailman/listinfo/bf-committers
