Looking for volunteers for clean work

[Simon Newton]

There are a number of small clean up tasks that would be good to close
out. If you're interested in getting involved but don't know where to
start, this may be a good opportunity. The tasks can be tackled in
chunks so we can have multiple people working on it if it comes to
that.

And you get your name in the authors file :).

Simon

[Sean Sill]

I may be able to commit to parts of this

[Peter Newman]

Is this to fix the Lint issues?

 

I can probably spend some time too.

[Simon Newton]

On Thu, May 9, 2013 at 5:14 AM, Peter Newman <peterj...@gmail.com> wrote:
> Is this to fix the Lint issues?

No, but we should add that to the list. So far I've got:

i) Migrate the remaining CPPUNIT_ASSERT to OLA_ASSERT
31 occurrences remaining by my count). Estimate 30 mins.

ii) Migrate binaries to the new flags module & large chunks of repeated code.
Commit 668d4adbc5631c02c8382bcf35507af10c44f6ff is a good example of what to do.
Estimate 30 mins - 1hr per binary for the first one, it probably drops
to 10 mins per binary once you get going.
Once we have this we can move command line parsing into ola::AppInit()
which reduces the work required to write new tools.

iii) Lint the codebase
This also involves coming up with a cpplint command line we can run in
the buildbot.
Estimate: unknown.


Feel free to reply "hey I'm working on X" and then other people won't
step on your toes.

Simon

> --
> The Open Lighting Group: open-l...@googlegroups.com, #openlighting
> (irc.freenode.org)
> To unsubscribe from this group, send email to
> open-lightin...@googlegroups.com
> For more options, visit https://groups.google.com/groups/opt_out?hl=en
>
>

[Joshua Moyerman]

For #3, Wouldn't a simple top level:
 $find *.cpp *.h | cpplint.py

work just fine, as long as the cpplint.py was added to the path

[Simon Newton]

On Thu, May 9, 2013 at 9:28 AM, Joshua Moyerman <j.moy...@gmail.com> wrote:
> For #3, Wouldn't a simple top level:
> $find *.cpp *.h | cpplint.py

The generated protobuf files aren't lint clean (nor will be unless we
start changing the generator) so they need to be excluded.

[Joshua Moyerman]

What about doing it before they're generated?
Otherwise it'd just take a bit of scripting to determine if the file is a protobuf one or not

[Sean Sill]

I'll take the change from CPPUNIT to OLA_ASSERT

[Peter Newman]

My hideous globbing command does work, and is already in the buildbot config.

I do find it rather ironic that Google's own generated C++ code doesn't pass it's own lint checker!

I guess we probably need a little bash script, that uses find to get any .cpp, .h and .cc files and then filters out stuff like the protobuffs we don't want to check. Ideally with an option to pass in a specific list of files we might need to check, so in future, the buildbot only needs to lint the files that have changed in this commit. On that slight tangent, the only .cc files we've currently got in the codebase are all .pb.cc, which I assume are all protobuffs, so this problem might have gone away, and just globbing/finding should work.

The lint builder actually runs in a separate work folder anyway, so make, and indeed configure are never run, so a lot of these issues are probably irrelevant anyway.

PN

[Simon Newton]

On Thu, May 9, 2013 at 5:52 PM, Sean Sill <sms...@gmail.com> wrote:
> I'll take the change from CPPUNIT to OLA_ASSERT

Thanks, have a look in
https://code.google.com/p/open-lighting/source/browse/include/ola/testing/TestUtils.h
for the new macros to use.

Simon

[Simon Newton]

On Thu, May 9, 2013 at 6:02 PM, Peter Newman <peterj...@gmail.com> wrote:
> My hideous globbing command does work, and is already in the buildbot
> config.
>
> I do find it rather ironic that Google's own generated C++ code doesn't pass
> it's own lint checker!
>
> I guess we probably need a little bash script, that uses find to get any
> .cpp, .h and .cc files and then filters out stuff like the protobuffs we
> don't want to check. Ideally with an option to pass in a specific list of
> files we might need to check, so in future, the buildbot only needs to lint
> the files that have changed in this commit. On that slight tangent, the only
> .cc files we've currently got in the codebase are all .pb.cc, which I assume
> are all protobuffs, so this problem might have gone away, and just
> globbing/finding should work.
>
> The lint builder actually runs in a separate work folder anyway, so make,
> and indeed configure are never run, so a lot of these issues are probably
> irrelevant anyway.

Yeah, that was Josh's suggestion which is perfectly reasonable. I
think with that there are only 790 lint errors. I may make a start on
fixing some of them as well.


Simon

[Simon Newton]

On Thu, May 9, 2013 at 7:55 PM, Simon Newton <nom...@gmail.com> wrote:
> On Thu, May 9, 2013 at 6:02 PM, Peter Newman <peterj...@gmail.com> wrote:
>> My hideous globbing command does work, and is already in the buildbot
>> config.
>>
>> I do find it rather ironic that Google's own generated C++ code doesn't pass
>> it's own lint checker!
>>
>> I guess we probably need a little bash script, that uses find to get any
>> .cpp, .h and .cc files and then filters out stuff like the protobuffs we
>> don't want to check. Ideally with an option to pass in a specific list of
>> files we might need to check, so in future, the buildbot only needs to lint
>> the files that have changed in this commit. On that slight tangent, the only
>> .cc files we've currently got in the codebase are all .pb.cc, which I assume
>> are all protobuffs, so this problem might have gone away, and just
>> globbing/finding should work.
>>
>> The lint builder actually runs in a separate work folder anyway, so make,
>> and indeed configure are never run, so a lot of these issues are probably
>> irrelevant anyway.
>
> Yeah, that was Josh's suggestion which is perfectly reasonable. I
> think with that there are only 790 lint errors. I may make a start on
> fixing some of them as well.

I've cleaned up the lint buildbot factory. Once we get some slaves
with cpplint installed we can enable it.

Simon

[Joshua Moyerman]

It's on mine.

[Andrew Frazer]

Tell me what needs installing on mirkwood and we can put it there as well.

[Joshua Moyerman]

$wget http://google-styleguide.googlecode.com/svn/trunk/cpplint/cpplint.py
$chmod +x cpplint.py
$mv cppylint.py /somewhere/in/your/path

[Simon Newton]

On Thu, May 9, 2013 at 8:45 PM, Joshua Moyerman <j.moy...@gmail.com> wrote:
> $wget http://google-styleguide.googlecode.com/svn/trunk/cpplint/cpplint.py
> $chmod +x cpplint.py
> $mv cppylint.py /somewhere/in/your/path

/usr/local/bin is a decent choice.

[Simon Newton]

On Thu, May 9, 2013 at 8:23 PM, Simon Newton <nom...@gmail.com> wrote:
> On Thu, May 9, 2013 at 7:55 PM, Simon Newton <nom...@gmail.com> wrote:
>> On Thu, May 9, 2013 at 6:02 PM, Peter Newman <peterj...@gmail.com> wrote:
>>> My hideous globbing command does work, and is already in the buildbot
>>> config.
>>>
>>> I do find it rather ironic that Google's own generated C++ code doesn't pass
>>> it's own lint checker!
>>>
>>> I guess we probably need a little bash script, that uses find to get any
>>> .cpp, .h and .cc files and then filters out stuff like the protobuffs we
>>> don't want to check. Ideally with an option to pass in a specific list of
>>> files we might need to check, so in future, the buildbot only needs to lint
>>> the files that have changed in this commit. On that slight tangent, the only
>>> .cc files we've currently got in the codebase are all .pb.cc, which I assume
>>> are all protobuffs, so this problem might have gone away, and just
>>> globbing/finding should work.
>>>
>>> The lint builder actually runs in a separate work folder anyway, so make,
>>> and indeed configure are never run, so a lot of these issues are probably
>>> irrelevant anyway.
>>
>> Yeah, that was Josh's suggestion which is perfectly reasonable. I
>> think with that there are only 790 lint errors. I may make a start on
>> fixing some of them as well.
>
> I've cleaned up the lint buildbot factory. Once we get some slaves
> with cpplint installed we can enable it.

Ok we have a lint slave now and I fixed a bunch of warnings. 385 to go!

Simon

[Andrew Frazer]

Down to 196 now. Good work folks.

[Joshua Moyerman]

I'll be hitting the ArtNet lint tomorrow and see where we get.

[Simon Newton]

21 now, they are all in the ArtNet code so I'll let Josh clean those up.

Simon

[Peter Newman]

I've started moving ola_dmxmonitor and ola_dmxconsole to the new flags module.

PN

[Simon Newton]

On Tue, May 14, 2013 at 10:36 AM, Peter Newman <peterj...@gmail.com> wrote:

I've started moving ola_dmxmonitor and ola_dmxconsole to the new flags module.

Cool, Sean's changed went in and the codebase is lint clean now so it's just the FLAGS changes to go.

Simon