[llvm-commits] [PATCH] [lld] patch for ELF Writer to get it to 'Hello world'

[Sean Silva]

>   I didnt want to make the patch more bigger by adding / changing existing
>   test cases.

You have this backward: a patch introducing this much new
functionality means that it *must* have a huge battery of tests (but
it shouldn't be this big in the first place).

>   This whole patch is needed to get 'Hello world' to work. I can split it up,
>   but will not get to the complete functionality by adding one at a time.

Just because a particular goal is tantalizingly close doesn't mean to
abandon proper incremental development and testing as is expected by
the LLVM community. A more incremental approach is easier to review
and would help to develop proper test cases.

This patch violates two core tenets of LLVM development style:
incremental development and strong testing. Please refresh your memory
about <http://llvm.org/docs/DeveloperPolicy.html#incremental-development>
and <http://llvm.org/docs/DeveloperPolicy.html#test-cases>.

-- Sean Silva

On Mon, Dec 17, 2012 at 7:59 PM, Shankar Kalpathi Easwaran
<shankarke at gmail.com> wrote:
>
>   Hi Michael, Sean
>
>   I am fixing the test cases, and I am planning to add a few to the list of
>   testcases.
>
>   I didnt want to make the patch more bigger by adding / changing existing
>   test cases.
>
>   Will do the changes after the review.
>
>   Sean,
>
>   This whole patch is needed to get 'Hello world' to work. I can split it up,
>   but will not get to the complete functionality by adding one at a time.
>
>   Thanks
>
>   Shankar Easwaran
>
> http://llvm-reviews.chandlerc.com/D222