lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Uwe Schindler (JIRA)" <>
Subject [jira] Commented: (LUCENE-2285) Code cleanup from all sorts of (trivial) warnings
Date Thu, 25 Feb 2010 08:28:28 GMT


Uwe Schindler commented on LUCENE-2285:

bq. I don't share the same feeling ... I think it's a strong capability - write a method which
doesn't need to start w/ testXYZ just to be run by JUnit (though I do both for clarity). I
think moving to JUnit 4 only simplifies things, as it allows testing classes w/o the need
to extend TestCase. But I'm not going to argue about it here, I'd like to keep this issue
contained, and short. So I won't touch the LuceneTestCase deprecation, as it's still controversial
judging by what you say. 

This was discussed lots of times in JIRA and frenode IRC (#lucene). The important thing is:
We want all testcases in lucene to be extended from one class (if Junit4 it's LuceneTestcaseJ4),
because we have some additional checks that should be run before/after each test: FieldCache
checkups, merge scheduler tests, reproducing random test seeds,...

bq. I'll remove those SuppressWarnings then?

Yes, I remove them only whenever i see them.

At the moment, such code cleanup  and if they also affect whitespace cleanup, have the problem
that they make merging with the new flex branch (flexible indexing) harder, so the most important
thing is to not simply reformat all the code. When adding generics, please use the above eclipse
codesyste, as we don't want to have whitespace after commas inside generics.

bq. About generics, there are the internal parts of the code, like using List, ArrayList etc.
Scanning quickly through the list, it looks like most of the Lucene related warnings are about
referencing them ... so it should be also easy to fix.

Last time I opened Lucene inside Eclipse only produced one type of warning: "Use of raw type"
- but this warning only affected instanceof checks. This is stupid and a bug in Eclipse. Instanceof
checks can never be generic, as the tests is useless at runtime. So a check for "instance
of ArrayList" without generics is perfectly fine. Adding generics make you feel that there
is some compiler check or runtime checks, both of them are not done. Instanceof is runtime-only
and can never check any generics. So I am against those checks! But if you find some unneeded
casts and missing generics in general, +1.

The instanceof warnings can somehow be switched off in Eclipse, dont know how. For coding
i dont use IDEs (only for automatic refactoring).

> Code cleanup from all sorts of (trivial) warnings
> -------------------------------------------------
>                 Key: LUCENE-2285
>                 URL:
>             Project: Lucene - Java
>          Issue Type: Improvement
>            Reporter: Shai Erera
>            Priority: Minor
>             Fix For: 3.1
> I would like to do some code cleanup and remove all sorts of trivial warnings, like unnecessary
casts, problems w/ javadocs, unused variables, redundant null checks, unnecessary semicolon
etc. These are all very trivial and should not pose any problem.
> I'll create another issue for getting rid of deprecated code usage, like LuceneTestCase
and all sorts of deprecated constructors. That's also trivial because it only affects Lucene
code, but it's a different type of change.
> Another issue I'd like to create is about introducing more generics in the code, where
it's missing today - not changing existing API. There are many places in the code like that.
> So, with you permission, I'll start with the trivial ones first, and then move on to
the others.

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message