search

apache / lucene

Apache Lucene open-source search software
https://lucene.apache.org/
Apache License 2.0
2.68k stars 1.04k forks source link

'ant javadocs' should fail if a package is missing a package.html [LUCENE-3887] #4960

Closed asfimport closed 12 years ago

asfimport commented 12 years ago

While reviewing the javadocs I noticed many packages are missing a basic package.html.

For 3.x I committed some package.html files where they were missing (I will port forward to trunk).

I think all packages should have this... really all public/protected classes/methods/constants, but this would be a good step.

Migrated from LUCENE-3887 by Robert Muir (@rmuir), resolved Apr 24 2012 Attachments: LUCENE-3887.patch (versions: 3)

asfimport commented 12 years ago

Michael McCandless (@mikemccand) (migrated from JIRA)

+1

It shouldn't be the RM who must do this on release...

asfimport commented 12 years ago

Robert Muir (@rmuir) (migrated from JIRA)

I couldn't find a 'javadocs linter' for stuff like this: does anyone know of one?

As a first step, maybe we could check this with some python code in the release smoke tester: sure its not ideal since its still stuck on the RM but its much better than manual eyeballing.

asfimport commented 12 years ago

Christian Moen (@cmoen) (migrated from JIRA)

Robert,

I should be careful recommending things I haven't tried, but perhaps Checkstyle (http://checkstyle.sourceforge.net/) could be useful for this. The javadoc package check described on http://checkstyle.sourceforge.net/config_javadoc.html seems to cover this sort of style check.

Checkstyle also integrates with build tools so it's also possible to do some light style checking as part of the build – either optional or integrated longer term, if we'd like that. (I'm sure there are various opinions on wether this is a good idea.)

asfimport commented 12 years ago

Robert Muir (@rmuir) (migrated from JIRA)

Christian this looks just like what we need! I like some of the other checks available on javadocs too.

One bummer is the GPL license, but maybe its ok for us to run this on hudson nightly as a separate thing somehow?

According to http://ant.apache.org/nightlies.html, they are using checkstyle.

asfimport commented 12 years ago

Jan Høydahl (@janhoy) (migrated from JIRA)

+1. Developers should have some ant-tasks that could be run to check that their change is compliant

Currently I run something "ant clean test javadocs dist" to validate things before checkin.

asfimport commented 12 years ago

Christian Moen (@cmoen) (migrated from JIRA)

Robert, very good point regarding the license. Thanks.

I believe it should be possible for us to run this as part of nightly builds somehow. If we do light checking only and expect few problems with code changes in general, I think this sort of scheme might work just fine.

Even if it turned out to be possible to include Checkstyle as a build-time dependency only by never including it in any binary or source distributions, I don't think doing so would be very wise.

Having said this, if we use Checkstyle as part of the nightly builds, there's soon a need to have it available for local builds as well, as Jan points out.

Perhaps running these checks could be an optional target for developers that they can do with their self-supplied Checkstyle jar? For example, it someone runs ant checkstyle, we'll tell them that Checkstyle isn't included for license reasons and quickly explain the few steps necessary to get checking going.

Personally, I think introducing more style checking in Lucene/Solr would be a good thing.

asfimport commented 12 years ago

Robert Muir (@rmuir) (migrated from JIRA)

Perhaps running these checks could be an optional target for developers that they can do with their self-supplied Checkstyle jar? For example, it someone runs ant checkstyle, we'll tell them that Checkstyle isn't included for license reasons and quickly explain the few steps necessary to get checking going.

Thats a good idea, maybe you just have to put the jar in your \~/.ant/lib otherwise the target does not work (kinda sorta similar to clover stuff we have).

asfimport commented 12 years ago

Shai Erera (@shaie) (migrated from JIRA)

In my projects, I handle stuff like that in unit tests. We can have a TestRelease unit test with testPackageHTML() that lists all directories and verifies that there's a package.html found.

We can have these sort of tests in some common misc/src folder, and have the build.xml always run them, whether you ran the test from a specific module, or from the root.

Just an idea.

asfimport commented 12 years ago

Michael McCandless (@mikemccand) (migrated from JIRA)

Simple patch to at least add basic checking to smokeTestRelease.py – it only checks for missing first-sentence description in package-summary.html.

asfimport commented 12 years ago

Michael McCandless (@mikemccand) (migrated from JIRA)

Another iteration, this time working I think :)

asfimport commented 12 years ago

Uwe Schindler (@uschindler) (migrated from JIRA)

Some unrelated fixes in the patch, otherwise ok for smokeTesting. I would just disagree to add python requirements to our official ant script...

asfimport commented 12 years ago

Robert Muir (@rmuir) (migrated from JIRA)

Uwe well we can discuss integration into the official ant build later?

For now personally I would like to have an automated check in the smokeTester script, that would help me clean the stuff up rather than manually eyeballing everything. Its a step.

asfimport commented 12 years ago

Uwe Schindler (@uschindler) (migrated from JIRA)

Did I say anything else?

asfimport commented 12 years ago

Michael McCandless (@mikemccand) (migrated from JIRA)

You can also just run the javadoc checker directly in a source checkout, like this:

  python -u dev-tools/scripts/checkJavaDocs.py /lucene/3x/lucene/build

You have to "ant javadocs" first yourself.

Right now it only checks for missing sentences in the package-summary.html... I'll see if I can fix it to also detect missing package.html's...

Here's what it reports on 3.x right now:

/lucene/3x/lucene/build/docs/api/contrib-highlighter/org/apache/lucene/search/highlight/package-summary.html
  missing: TokenStreamFromTermPositionVector

/lucene/3x/lucene/build/docs/api/contrib-highlighter/org/apache/lucene/search/vectorhighlight/package-summary.html
  missing: BoundaryScanner
  missing: BaseFragmentsBuilder
  missing: FieldFragList.WeightedFragInfo
  missing: FieldFragList.WeightedFragInfo.SubInfo
  missing: FieldPhraseList.WeightedPhraseInfo
  missing: FieldPhraseList.WeightedPhraseInfo.Toffs
  missing: FieldQuery.QueryPhraseMap
  missing: FieldTermStack.TermInfo
  missing: ScoreOrderFragmentsBuilder.ScoreComparator
  missing: SimpleBoundaryScanner

/lucene/3x/lucene/build/docs/api/contrib-spatial/org/apache/lucene/spatial/tier/package-summary.html
  missing: DistanceHandler.Precision

/lucene/3x/lucene/build/docs/api/contrib-spellchecker/org/apache/lucene/search/suggest/package-summary.html
  missing: Lookup.LookupPriorityQueue

/lucene/3x/lucene/build/docs/api/contrib-spellchecker/org/apache/lucene/search/suggest/jaspell/package-summary.html
  missing: JaspellLookup

/lucene/3x/lucene/build/docs/api/contrib-spellchecker/org/apache/lucene/search/suggest/tst/package-summary.html
  missing: TSTAutocomplete
  missing: TSTLookup

/lucene/3x/lucene/build/docs/api/contrib-pruning/org/apache/lucene/index/pruning/package-summary.html
  missing: CarmelTopKTermPruningPolicy.ByDocComparator
  missing: CarmelUniformTermPruningPolicy.ByDocComparator

/lucene/3x/lucene/build/docs/api/contrib-facet/org/apache/lucene/facet/taxonomy/writercache/lru/package-summary.html
  missing: LruTaxonomyWriterCache.LRUType

/lucene/3x/lucene/build/docs/api/contrib-facet/org/apache/lucene/facet/index/package-summary.html
  missing: FacetsPayloadProcessorProvider.FacetsDirPayloadProcessor

/lucene/3x/lucene/build/docs/api/core/org/apache/lucene/store/package-summary.html
  missing: FSDirectory.FSIndexOutput
  missing: NIOFSDirectory.NIOFSIndexInput
  missing: RAMFile
  missing: SimpleFSDirectory.SimpleFSIndexInput
  missing: SimpleFSDirectory.SimpleFSIndexInput.Descriptor

/lucene/3x/lucene/build/docs/api/core/org/apache/lucene/index/package-summary.html
  missing: MergePolicy.MergeAbortedException

/lucene/3x/lucene/build/docs/api/core/org/apache/lucene/search/package-summary.html
  missing: FieldCache.CreationPlaceholder
  missing: FieldComparator.NumericComparator<T extends Number>
  missing: FieldValueHitQueue.Entry
  missing: QueryTermVector
  missing: ScoringRewrite<Q extends Query>
  missing: SpanFilterResult.PositionInfo
  missing: SpanFilterResult.StartEnd
  missing: TimeLimitingCollector.TimerThread

/lucene/3x/lucene/build/docs/api/core/org/apache/lucene/util/package-summary.html
  missing: ByteBlockPool.Allocator
  missing: ByteBlockPool.DirectAllocator
  missing: ByteBlockPool.DirectTrackingAllocator
  missing: BytesRefHash.BytesStartArray
  missing: BytesRefHash.DirectBytesStartArray
  missing: BytesRefIterator.EmptyBytesRefIterator
  missing: DoubleBarrelLRUCache.CloneableKey
  missing: OpenBitSetDISI
  missing: PagedBytes.Reader
  missing: UnicodeUtil.UTF16Result
  missing: UnicodeUtil.UTF8Result

/lucene/3x/lucene/build/docs/api/contrib-analyzers/org/tartarus/snowball/package-summary.html
  missing: Among
  missing: TestApp

/lucene/3x/lucene/build/docs/api/contrib-xml-query-parser/org/apache/lucene/xmlparser/package-summary.html
  missing: FilterBuilder
  missing: CorePlusExtensionsParser
  missing: DOMUtils
  missing: FilterBuilderFactory
  missing: QueryBuilderFactory
  missing: ParserException

/lucene/3x/lucene/build/docs/api/contrib-xml-query-parser/org/apache/lucene/xmlparser/builders/package-summary.html
  missing: SpanQueryBuilder
  missing: BooleanFilterBuilder
  missing: BooleanQueryBuilder
  missing: BoostingQueryBuilder
  missing: BoostingTermBuilder
  missing: ConstantScoreQueryBuilder
  missing: DuplicateFilterBuilder
  missing: FilteredQueryBuilder
  missing: FuzzyLikeThisQueryBuilder
  missing: LikeThisQueryBuilder
  missing: MatchAllDocsQueryBuilder
  missing: RangeFilterBuilder
  missing: SpanBuilderBase
  missing: SpanFirstBuilder
  missing: SpanNearBuilder
  missing: SpanNotBuilder
  missing: SpanOrBuilder
  missing: SpanOrTermsBuilder
  missing: SpanQueryBuilderFactory
  missing: SpanTermBuilder
  missing: TermQueryBuilder
  missing: TermsFilterBuilder

/lucene/3x/lucene/build/docs/api/contrib-misc/org/apache/lucene/store/package-summary.html
  missing: NativePosixUtil
  missing: WindowsDirectory.WindowsIndexInput

/lucene/3x/lucene/build/docs/api/test-framework/org/apache/lucene/store/package-summary.html
  missing: MockDirectoryWrapper.Throttling

/lucene/3x/lucene/build/docs/api/test-framework/org/apache/lucene/util/package-summary.html
  missing: English
  missing: StoreClassNameRule
  missing: SystemPropertiesInvariantRule
  missing: UncaughtExceptionsRule.UncaughtExceptionEntry

/lucene/3x/lucene/build/docs/api/contrib-benchmark/org/apache/lucene/benchmark/byTask/feeds/demohtml/package-summary.html
  missing: Entities
  missing: HTMLParser
  missing: Tags

/lucene/3x/lucene/build/docs/api/contrib-benchmark/org/apache/lucene/benchmark/byTask/tasks/package-summary.html
  missing: BenchmarkHighlighter
  missing: NewCollationAnalyzerTask.Implementation

/lucene/3x/lucene/build/docs/api/contrib-queries/org/apache/lucene/search/package-summary.html
  missing: DuplicateFilter

/lucene/3x/lucene/build/docs/api/contrib-queryparser/org/apache/lucene/queryParser/core/nodes/package-summary.html
  missing: TextableQueryNode
  missing: PathQueryNode.QueryText
  missing: PhraseSlopQueryNode
  missing: ProximityQueryNode.ProximityType
  missing: ModifierQueryNode.Modifier
  missing: ParametricQueryNode.CompareOperator
  missing: ProximityQueryNode.Type

/lucene/3x/lucene/build/docs/api/contrib-queryparser/org/apache/lucene/queryParser/core/parser/package-summary.html
  missing: EscapeQuerySyntax.Type

/lucene/3x/lucene/build/docs/api/contrib-queryparser/org/apache/lucene/queryParser/surround/query/package-summary.html
  missing: DistanceSubQuery
  missing: SimpleTerm.MatchingTermVisitor
  missing: AndQuery
  missing: BasicQueryFactory
  missing: ComposedQuery
  missing: DistanceQuery
  missing: FieldsQuery
  missing: NotQuery
  missing: OrQuery
  missing: SimpleTerm
  missing: SpanNearClauseFactory
  missing: SrndPrefixQuery
  missing: SrndQuery
  missing: SrndTermQuery
  missing: SrndTruncQuery
  missing: TooManyBasicQueries

/lucene/3x/lucene/build/docs/api/contrib-queryparser/org/apache/lucene/queryParser/standard/parser/package-summary.html
  missing: EscapeQuerySyntaxImpl
  missing: StandardSyntaxParser

/lucene/3x/lucene/build/docs/api/contrib-queryparser/org/apache/lucene/queryParser/standard/config/package-summary.html
  missing: FuzzyConfig
  missing: StandardQueryConfigHandler.ConfigurationKeys
  missing: DefaultOperatorAttribute.Operator
  missing: StandardQueryConfigHandler.Operator

/lucene/3x/lucene/build/docs/api/contrib-queryparser/org/apache/lucene/queryParser/standard/builders/package-summary.html
  missing: AnyQueryNodeBuilder
asfimport commented 12 years ago

Michael McCandless (@mikemccand) (migrated from JIRA)

OK I committed the basic checking for smoke tester...

I'll leave this open for having "ant javadocs" fail when things are missing...

asfimport commented 12 years ago

Robert Muir (@rmuir) (migrated from JIRA)

initial patch: just hooks into lucene's 'javadocs-lint' (but also a one-liner to hook into solrs).

I added an option for 'level' of checking, currently its at 'package'. the idea is we fix those... move to 'class'.... then 'method'....

asfimport commented 12 years ago

Robert Muir (@rmuir) (migrated from JIRA)

This is fixed: but it would be good to fix it at class level as well, and also good to at least have package.htmls for Solr and enable the check there.