Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1)

Issue 3480004: Change Makefile for new test ebuild. (Closed)

Created:
10 years, 3 months ago by Luigi Semenzato
Modified:
9 years, 7 months ago
Reviewers:
zbehan
CC:
chromium-os-reviews_chromium.org, Randall Spangler, gauravsh, Bill Richardson
Visibility:
Public.

Description

Change Makefile for new test ebuild. Change-Id: Idb081dccdcba17005cd3edc059e58b78316c3dbe BUG=none TEST=ran the ebuild (separate CL) and verified that the targets are created Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=e19da8b

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+17 lines, -27 lines) Patch
M autotest/client/hardware_TPMFirmware/src/Makefile View 1 chunk +17 lines, -27 lines 1 comment Download

Messages

Total messages: 4 (0 generated)
Luigi Semenzato
Thanks!
10 years, 3 months ago (2010-09-21 16:12:30 UTC) #1
zbehan
This looks like an entire rewrite. I assume the test was not used very much ...
10 years, 3 months ago (2010-09-21 17:48:18 UTC) #2
Luigi Semenzato
That is correct, I changed the tests quite a bit recently. I like leaving the ...
10 years, 3 months ago (2010-09-21 18:03:38 UTC) #3
zbehan
10 years, 3 months ago (2010-09-21 18:07:40 UTC) #4
SGTM; LGTM

On Tue, Sep 21, 2010 at 11:03 AM, <semenzato@chromium.org> wrote:

> That is correct, I changed the tests quite a bit recently.
>
> I like leaving the \ at the end of every test for uniformity, so that if I
> delete the last test, or add one more after it, it's a one-line change.  Of
> course this means that the following line must be blank, but that usually
> works
> well.
>
>
> On 2010/09/21 17:48:18, zbehan wrote:
>
>> This looks like an entire rewrite. I assume the test was not used very
>> much
>>
> and
>
>> the list of binaries is now outdated, yes? :)
>>
>
>  Anyway, LGTM with nit
>>
>
>  http://codereview.chromium.org/3480004/diff/1/2
>> File autotest/client/hardware_TPMFirmware/src/Makefile (right):
>>
>
>  http://codereview.chromium.org/3480004/diff/1/2#newcode17
>> autotest/client/hardware_TPMFirmware/src/Makefile:17: writelimit       \
>> No trailing \ here needed.
>>
>
>
>
> http://codereview.chromium.org/3480004/show
>

Powered by Google App Engine
This is Rietveld 408576698