[
https://issues.apache.org/jira/browse/LUCENE-7788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17084423#comment-17084423
]
Erick Erickson edited comment on LUCENE-7788 at 4/15/20, 11:02 PM:
-------------------------------------------------------------------
My current thinking:
1> I think I've got the gradle task in place, sometime in the next couple of
days I'll put up a preliminary version and ask for review
2> Given that the Lucene code only takes a couple of seconds to run, I I'll
leave it in
3> My current Gradle integration _requires_ a relative path, i.e. "gradlew
validateLoggingCalls -Psolr/core/src/java/org/apache/solr/response". Before
it's done, I'll change it to "the gradle way" of specifying a project rather
than a directory, defaulting to all. But right now projects are too big.
3a> really, if the path to any java file anywhere contains whatever srcDir is,
it'll check the file. This is temporary so there's no need to refine it IMO.
4> this is not part of the standard check/precommit yet. It will be before I'm
done.
5> I'm not happy at all with the //verify tag. First of all, exactly when it's
OK to use it isn't clear at all. So I've changed my mind (again) and I'll
change the check to be that the call must not have "+" signs or method calls
_unless_ it's surrounded by "if (log.is*Enabled)". I think that's a much easier
rule to understand. I'll also add a check that the log level corresponds to the
if clause when used.
was (Author: erickerickson):
My current thinking:
1> I think I've got the gradle target in place, sometime in the next couple of
days I'll put up a preliminary version and ask for review
2> Given that the Lucene code only takes a couple of seconds to run, I I'll
leave it in
3> My current Gradle integration _requires_ a relative path, i.e. "gradlew
validateLoggingCalls -Psolr/core/src/java/org/apache/solr/response". Before
it's done, I'll change it to "the gradle way" of specifying a project rather
than a directory, defaulting to all. But right now projects are too big.
3a> really, if the path to any java file anywhere contains whatever targetDir
is, it'll check the file. This is temporary so there's no need to refine it IMO.
4> this is not part of the standard check/precommit yet. It will be before I'm
done.
5> I'm not happy at all with the //verify tag. First of all, exactly when it's
OK to use it isn't clear at all. So I've changed my mind (again) and I'll
change the check to be that the call must not have "+" signs or method calls
_unless_ it's surrounded by "if (log.is*Enabled)". I think that's a much easier
rule to understand. I'll also add a check that the log level corresponds to the
if clause when used.
> fail precommit on unparameterised log messages and examine for wasted
> work/objects
> ----------------------------------------------------------------------------------
>
> Key: LUCENE-7788
> URL: https://issues.apache.org/jira/browse/LUCENE-7788
> Project: Lucene - Core
> Issue Type: Task
> Reporter: Christine Poerschke
> Assignee: Erick Erickson
> Priority: Minor
> Attachments: LUCENE-7788.patch, LUCENE-7788.patch
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> SOLR-10415 would be removing existing unparameterised log.trace messages use
> and once that is in place then this ticket's one-line change would be for
> 'ant precommit' to reject any future unparameterised log.trace message use.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
