<p dir="ltr"><br>
On Feb 26, 2013 2:20 AM, "Christian König" <<a href="mailto:deathsimple@vodafone.de">deathsimple@vodafone.de</a>> wrote:<br>
><br>
> Am 26.02.2013 03:33, schrieb Sean Silva:<br>
>><br>
>> On Mon, Feb 25, 2013 at 3:00 PM, Tom Stellard <<a href="mailto:tom@stellard.net">tom@stellard.net</a>> wrote:<br>
>>><br>
>>> Can you clarify this?  Do you mean one email with with all the patches<br>
>>> as attachments?<br>
>><br>
>><br>
>> That would be preferable. Note that gmail (which many of us use) doesn't respect threading (instead it arbitrarily threads based on subject for some reason), so having them all as replies will still flood the inbox of gmail users.<br>

>><br>
><br>
> That's the exactly opposite policy as many other open source development list use, the argument pro in-lining patches is usually that the code is easier to comment on. </p>
<p dir="ltr">Yep, different communities have different processes. I don't personally mind a combination of both (attachment and inline) though one of the reasons we prefer to have the attachment (even if there's also an inline copy) is that it's authoritative. All whitespace is preserved, no email server induced line wrapping, etc (and we're kind of pedantic about formatting, trailing whitespace, etc, so it's important to have an immutable, exact copy)</p>

<p dir="ltr">Alternatively, we do have the phabricator code review system setup to assist reviewing too. (Which provides some of the benefits of both inline and authoritative patch content)</p>
<p dir="ltr">> But personally I usually also prefer some form of one mail (or one thread) per patchset for review. </p>
<p dir="ltr">The issue of what to do with a patchset is still a bit open/uncertain in the llvm community. I for one don't /entirely/ mind one email per patch because it can be easier to track which parts of the set have been resolved/committed.</p>

<p dir="ltr">><br>
>> However, it is much more in line with LLVM development style to send in patches incrementally for review rather than dumping a whole branch; that ensures focused review.<br>
><br>
><br>
> Those patches belong together and either implement new features or fix bugs that were triggered/found while implementing those new features. Reviewing them separately would just increase the chance of missing something regarding inter patch dependency.</p>

<p dir="ltr">There should not be circular dependency, one way dependence should be resolved by sending the dependent patches first, as they are developed. They should stand in their own right - though may require some narrative to explain where they fit in a longer term plan.</p>

<p dir="ltr">Though of course this is not always the case and sometimes you don't know where a patch set will start until you get to the end. In which case you have a whole patch set ready for review/commit at the same time. Sending that in one batch is what it is - but when possible it's nice to avoid such a dump.</p>

<p dir="ltr">><br>
><br>
>> (I'll also reiterate how troubling it is that none of these R600 changes have unit tests).<br>
><br>
><br>
> Totally agree on that, the major problem seems to be that currently the instruction scheduler and/or register allocator produce different results when compiling the same code twice. The results seems to be always correct, it's just not very well comparable. That fact makes it quite difficult for me to write profound unit tests. Any ideas on that?</p>

<p dir="ltr">Any case of nondeterminism is a serious bug. Please provide repro steps.</p>
<p dir="ltr">><br>
> Christian.<br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
</p>