lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Shai Erera (JIRA)" <j...@apache.org>
Subject [jira] Commented: (LUCENE-2285) Code cleanup from all sorts of (trivial) warnings
Date Fri, 26 Feb 2010 19:01:29 GMT

    [ https://issues.apache.org/jira/browse/LUCENE-2285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12838983#action_12838983
] 

Shai Erera commented on LUCENE-2285:
------------------------------------

Robert - I actually thought about whether or not to change the Snowball code. In my internal
project, I also make use of Snowball code directly, and improved it to fit better in the analysis
process. I should actually diff my changes w/ yours, perhaps I can use yours, or contribute
mine. But all I did is get rid of using deprecated methods. I didn't remove any casts (I think?),
but actually added ones in order to not use the deprecated API.

But from what you say I understand why you prefer to leave the code as-is. It'll make comparison
w/ the source Snowball easier. So I'm fine w/ leaving this part out. Perhaps we can just add
a SuppressWarning to not see the deprecated warnings? That should be fairly easy to skip over
when comparing the sources in the future.

Uwe/Michael - your reasoning about QueryParser makes sense to me. I have a Tokenizer (well
few actually) generated by JFlex and I clean up warnings as well. But that's me, where I have
full control over the code (and I don't mean just commit rights) - if anything gets rebuilt,
it's because I've decided to do it. Therefore I'm ok w/ leaving these out. I think what I've
added to QueryParser though is a general SuppressWarnings above the class declaration (in
the .jj file). That shouldn't be left out right? I mean, what problems can it cause? But if
you feel otherwise, I won't stop you :).

TEST_VERSION_CURRENT - I think if it's already included in that patch why not commit it here?
I'm not looking for credit or anything, just to save Uwe's work. Separating it out from this
patch and including it over in the other issue is just extra work ... but Uwe - it's your
time that's spent, so I won't tell you how to manage it ;).

Uwe, about the 'important' casts, my preference is to include a suitable comment. I.e., if
you have a code that looks like *long val = (long) otherval* just to avoid overflow, then
that's not clear anyway. Someone can still change it to *int val = otherval* w/o knowing/understanding
that he just broke something. These types of casts are anyhow redundant as I don't believe
someone will change them. Nowadays, w/ 64-bit machines, compilers and OSs, keeping your stuff
as long does not matter much (I mean as variables, not as elements in an array).

It'll be interesting to see how this patch turns out eventually. All I cared about is reducing
the number of warnings. If we can keep the Snowball/QueryParser stuff under a SuppressWarnings
annotations, then that will do the trick as well :).

Thanks everybody for looking into this !

> Code cleanup from all sorts of (trivial) warnings
> -------------------------------------------------
>
>                 Key: LUCENE-2285
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2285
>             Project: Lucene - Java
>          Issue Type: Improvement
>            Reporter: Shai Erera
>            Assignee: Uwe Schindler
>            Priority: Minor
>             Fix For: 3.1
>
>         Attachments: LUCENE-2285.patch, LUCENE-2285.patch
>
>
> 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: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org


Mime
View raw message