r213171 - Make clang's rewrite engine a core feature

[Nico Weber]

On Fri, Jul 18, 2014 at 10:18 AM, Sean Silva <chisophugis at gmail.com> wrote:

>
>
>
> On Thu, Jul 17, 2014 at 3:43 PM, Alp Toker <alp at nuanti.com> wrote:
>
>>
>> On 18/07/2014 00:13, Douglas Gregor wrote:
>>
>>> On Jul 17, 2014, at 4:24 AM, Alp Toker <alp at nuanti.com> wrote:
>>>>
>>>>
>>>> On 17/07/2014 09:21, Richard Smith wrote:
>>>>
>>>>> On Wed, Jul 16, 2014 at 8:07 PM, Alp Toker <alp at nuanti.com <mailto:
>>>>> alp at nuanti.com>> wrote:
>>>>>
>>>>>          My reply here was me taking my earliest opportunity to engage
>>>>>         and discuss. =)
>>>>>
>>>>>
>>>>>     I don't understand your sudden expectation to be copied into the
>>>>>     discussion here. It's a module you haven't committed to at all
>>>>>     recently or as far back as I can remember. I've been relatively
>>>>>     active in this corner of the codebase and the change was discussed
>>>>>     and approved on the commits list.
>>>>>
>>>>>
>>>>> There were two changes here. One (the file reorganization) was not
>>>>> discussed and not approved.
>>>>>
>>>> It's normal to reformat code, reorganise files, fix up style in areas
>>>> you're working on. This change had no functional impact at all and is
>>>> purely a cleanup, and you said yourself it looks to be an improvement.
>>>>
>>> Yes, but it’s a matter of degrees. Reforming some code, splitting one
>>> .cpp file into multiple .cpp files, renaming a file whose purpose has
>>> shifted… these are all normal cleanup activities. Moving and renaming a
>>> library with more than 10 source files, and which has been in Clang for a
>>> very long time and has external clients, is a more significant change
>>> bordering on architectural. It’s reasonable to request/expect some
>>> discussion before committing the latter.
>>>
>>>  For the other, only two hours passed between the first suggestion of
>>>>> the change and the commit,
>>>>>
>>>> Two hours is a comparatively long window.
>>>>
>>>> People at your organisation frequently commit LLVM and clang patches
>>>> within 5-15 minutes of a colleague posting the patch:
>>>>
>>>> Every day commits get pushed to SVN before the mailing list has even
>>>> *forwarded* the original patch, so I find a complaint about a lengthy two
>>>> hour window hard to grasp. This is totally not going to change.
>>>>
>>> Nor should it, but we do discuss more-significant changes first, and two
>>> hours is too short for more-significant changes.
>>>
>>>  the discussion happened on an unrelated patch review thread,
>>>>>
>>>> The other thread was entirely related to this change. It also hit the
>>>> same keywords about "moving files" and "rewrite" and enabling features so
>>>> the thread had complete topical overlap, and an equivalent audience. It
>>>> would have been most unconventional to split the thread.
>>>>
>>> One could very easily have skipped the previous thread as “not
>>> interesting” because its subject did not imply as large a change as it
>>> ended up implying. It’s reasonable to ask that changes on this scale get
>>> their own threads with a subject that implies the scale of the change.
>>>
>>>  The level and duration of preceding discussion was not sufficient for
>>>>> either of these changes.
>>>>>
>>>> If you want to introduce a mandatory review window, please propose that
>>>> as an amendment to the developer policy on llvmdev. There's nothing
>>>> actionable I can change based on this discussion and I need to make it
>>>> clear that I believe we've followed best practice here. The mailing list
>>>> workflow is central to the project and that needs to be said loud and clear.
>>>>
>>>
>>> Obviously, mandatory review windows don’t make sense, and haven’t
>>> actually been proposed, so let’s ignore that bit.
>>>
>>> Fundamentally, we agree that small things should be post-commit—reviewed
>>> and that larger things should have some discussion first. There is no
>>> hard-and-fast rule we can come up with to separate “small” from “larger”
>>> things. In a fast-moving community, that means we’ll see disagreement from
>>> time to time.
>>>
>>
>> Sure, but let's hold the rest of the community to those standards too?
>>
>> Right now there's a lot of commit activity that's not going through the
>> list at all and which needs at least as much scrutiny as a reorg commit
>> that actually had a two hour window on the list.
>
>
> If what you're saying is correct then let's discuss in a focused -dev
> thread.
>

I'm also wondering what this refers to.


>
>
> -- Sean Silva
>
>
>>
>>
>>
>>
>>  The advice here is that renaming libraries and moving a dozen source
>>> files is considered “larger” and needs some mention on the mailing lists,
>>> with its own separate thread, that gives others a day or so to respond.
>>>
>>>         - Doug
>>>
>>>
>> --
>> http://www.nuanti.com
>> the browser experts
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140718/c2959020/attachment.html>