[osmosis-dev] TagFilter

[Brett Henderson]

On Thu, Dec 3, 2009 at 10:50 PM, Andrew Byrd <andrew at fastmail.net> wrote:

>
> On 03 Dec 2009, at 12:35, Brett Henderson wrote:
>
>>
>> Hi Andrew,
>>
>> I've just tried to apply the patch but ran into a few problems.
>> * Can you please run "ant checkstyle" and fix any errors that pop up?
>>  These errors are currently breaking the build.  Most are code formatting
>> issues and should be fairly easy to fix.  You can configure eclipse to
>> highlight checkstyle issues if you install the checkstyle eclipse plugin.
>> * There is a compilation warning.  At least there is in Eclipse.  There is
>> a "private Class filterClass" variable in TagFilter that has a warning
>> because Class is a generic type and no type is specified.  You can fix it
>> with something like "private Class<? extends EntityContainer> filterClass".
>> * Code should never write to System.out or System.err.  System.out in
>> particular is bad because it corrupts data if piping data between apps.  The
>> debug statements you referred to can be kept but change them to use JDK
>> Logging.  You can use a class like PostgreSqlDatasetTruncator as an example.
>>  The type of logging you're writing out would probably be suitable for a
>> "finer" level of logging.
>> * Do you have any unit tests?  Nothing too extravagant is required, but a
>> couple of simple tests would be very helpful.
>>
>
> OK, thanks for the pointers. I'll look into all of the above. I don't
> usually program in Java, so some of this (formatting/style issues) I'm
> picking up as I go along. Developing some tests should also reassure me that
> everything works as expected - I was a little wary of inorporating this into
> the repository without more testing.
>

Yep, that's all cool.  Ideally I'd like everybody to run a full build
locally before committing anything but it's tricky at the moment due to some
tests requiring access to databases.  As a result it's hard to know if
you've missed anything.  I'm hoping to get a continuous integration server
going one of these days to at least notify the osmosis-dev list when
something has broken.  For now, if you can run an "ant dist" and "ant
checkstyle" and your own unit tests successfully you should be almost there.

There's no need to go too overboard with testing, I don't expect anything
too comprehensive.  If your implementation has some bugs it's not a huge
deal so long as you're prepared to help fix them as they arise.  The real
value in the tests comes later on when changes are made to the codebase and
regression tests are needed to make sure things haven't been broken.

Brett
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/osmosis-dev/attachments/20091203/db2c8358/attachment.html>