[LLVMdev] [llvm] r210424 - Revert "Do materialize for floating point"

[Rafael Espíndola]

On 8 June 2014 05:43, Reed Kotler <rkotler at mips.com> wrote:
> Why are you reverting patches for any area that you have no authorization
> for ?
>
> No build was broken. This patch is fine.
>
> I am authorized to check in to the Mips area and Daniel is the maintainer
> for that area.

Hi Reed,

I hadn't noticed this patch until now, so this is mostly about
protocol, not my opinion about the patch.

Code ownership in llvm is a statement more about responsibility than
actual authority. Think of it as owning the code review, not the code
proper. It is the code owner responsibility to make sure the reviews
run correctly for a particular section, but there is no special
permission for cutting out others from the review process or other
shortcuts.

In this particular case it looks like some review feedback was
ignored, at least in the public list. For example, if the isa x
dyn_cast issue is a pattern over all of the llvm code base. So if a
misuse was noticed during code review, it should be handled. If
outside of the list it was decided that the original patch was
actually correct, it is good practice to say something on commit
message about it.

Same goes for having a commit message more in line with what we do in
llvm. Looking at it now I see that it has paragraphs that are not even
full sentences ("formatting"). What is being formatted? Why is that
not an independent patch?

So it does look like we will be better off with the patch reverted and
recommitted, but then with a better explanation of what is going on
and the llvm wide issues (isa X dyn_cast, incremental commits, etc)
fixed.

Cheers,
Rafael