[PATCH] fix llvm linker issue with dbg.declare

[Nick Lewycky]

Bob Wilson wrote:
> Eli,
>
> This is not a new patch review policy. It is the old policy:
>
> LLVM has a code review policy. Code review is one way to increase the
> quality of software. We generally follow these policies:
>
>  1. All developers are required to have significant changes reviewed
>     before they are committed to the repository.
>  2. Code reviews are conducted by email, usually on the llvm-commits list.
>  3. Code can be reviewed either before it is committed or after. We
>     expect major changes to be reviewed before being committed, but
>     smaller changes (or changes where the developer owns the component)
>     can be reviewed after commit.
>  4. The developer responsible for a code change is also responsible for
>     making all necessary review-related changes.
>  5. Code review can be an iterative process, which continues until the
>     patch is ready to be committed.
>
> I specifically asked Manman to commit this without waiting for a review,
> because I do not consider it to be a "significant change".

I want to make a quick point here, as there's something unclear about 
this line which concerns me.

The right way to convince someone to commit a patch is by reviewing it. 
Even when you are not an expert in the area, the fact both of you have 
looked at it can raise your confidence in a patch up to the "good enough 
for post-commit review" bar.

But, please don't ever let me hear that someone has committed a patch 
they still think needs pre-commit review, or otherwise shouldn't be 
committed, just because their manager has asked them to. That wouldn't 
be okay.

Nick

  It was nice
> of her to ask for a pre-commit review, but I did not think that was
> necessary. There have been a lot of new contributors to LLVM recently,
> and we have consequently been asking for more pre-commit reviews, but in
> general, experienced LLVM developers can go with post-commit reviews for
> changes like this. Manman has been actively contributing to LLVM for
> almost a year, so I think we can trust her.

>
> On Jan 30, 2013, at 10:52 AM, Chad Rosier <mcrosier at apple.com
> <mailto:mcrosier at apple.com>> wrote:
>
>>
>> On Jan 30, 2013, at 10:37 AM, Eli Bendersky <eliben at google.com
>> <mailto:eliben at google.com>> wrote:
>>
>>>>> Since I got no response on this patch and we need this patch soon,
>>>>> I committed at r173946.
>>>>
>>>> Manman,
>>>> It may have been best to continue pressing for a review first.
>>>>
>>>
>>> Yes.
>>>
>>>>>> If you have any concern about this patch, please review after commit.
>>>>>>
>>>>>
>>>>> Ah, it's good to know there is a new patch review policy for LLVM.
>>>>> Thanks, Manman.
>>>>
>>>> Eli,
>>>> What was the point of this comment?
>>>
>>> The point was to express the same thing you said above. The sarcastic
>>> tone may have been uncalled for, but stems from bitter experience.
>>
>> I'm sorry you've had a number of poor experiences, but lets try to be
>> constructive/positive despite them.
>>
>>> Having had the pleasure (on multiple occasions for multiple code
>>> areas) to chase code owners for weeks with multiple list & IRC pings
>>> before getting the LGTM, it hurts to see the honor code broken by a
>>> review-less commit made less than 48 hours after the initial request,
>>> because "we need this patch soon".
>>
>> Thanks for the clarification, Eli. I'm sure it's very frustrating.
>>
>>> Eli
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits