Facet examples javadoc missing in 4.0?

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

Facet examples javadoc missing in 4.0?

Shai Erera
Hi

I noticed that the javadocs for the facet module do not exist in 4.0. In the past, the facet/build.xml contains a <javadoc> tag with these packages:

          <packageset dir="${src.dir}"/>
          <packageset dir="${examples.dir}"/>

It seems that with the refactoring of the modules-javadocs build system, they were removed and now every module contains only the "src.dir" packageset.

I didn't find a clean way to add another package source to the facet javadocs, besides overriding <javadocs> and inline <invoke-javadoc-module>, to add another packagset.

Is that indeed the best approach, or can <invoke-javadoc-module> be extended to support additional sources?

Shai
Reply | Threaded
Open this post in threaded view
|

Re: Facet examples javadoc missing in 4.0?

Robert Muir


On Sat, Nov 24, 2012 at 2:13 AM, Shai Erera <[hidden email]> wrote:
Hi

I noticed that the javadocs for the facet module do not exist in 4.0. In the past, the facet/build.xml contains a <javadoc> tag with these packages:

          <packageset dir="${src.dir}"/>
          <packageset dir="${examples.dir}"/>

It seems that with the refactoring of the modules-javadocs build system, they were removed and now every module contains only the "src.dir" packageset.

I didn't find a clean way to add another package source to the facet javadocs, besides overriding <javadocs> and inline <invoke-javadoc-module>, to add another packagset.

Is that indeed the best approach, or can <invoke-javadoc-module> be extended to support additional sources?


I don't think we should do this.

Can we merge these examples into the demo/ module?
Otherwise lets make a facet-examples module, if it wants to have its own jar/javadocs/etc.

Reply | Threaded
Open this post in threaded view
|

Re: Facet examples javadoc missing in 4.0?

Shai Erera
bq. I don't think we should do this.

Why? What's the harm? It's a fairly simple fix to the build.xml and it's not like all other modules have example code under demo, such that the facet one stands out.

bq. Can we merge these examples into the demo/ module?

We tried that in LUCENE-3998 without success. The problem are facet tests which depend on data that exists in the example package. I don't want to disable tests just for the sake of separation, nor move to the examples package tests that should reside in the module itself.

I don't think that it's easy to make this separation currently, but we can discuss it more on the issue. I created a patch then with my attempts, which were no successful, I'll try to find it.

In the meantime, I want to fix the build.xml. If we'll ever move examples under demo, we'll notice it right away (the build won't find the src/examples dir under facet) and we can remove the inlining of invoke-module-javadoc.

Or, we just open up the possibility for that macro to take additional packages, but I don't know how to do that. I tried to define <sources> in facet/build.xml, but it didn't like it ...

Shai

On Sat, Nov 24, 2012 at 3:28 PM, Robert Muir <[hidden email]> wrote:


On Sat, Nov 24, 2012 at 2:13 AM, Shai Erera <[hidden email]> wrote:
Hi

I noticed that the javadocs for the facet module do not exist in 4.0. In the past, the facet/build.xml contains a <javadoc> tag with these packages:

          <packageset dir="${src.dir}"/>
          <packageset dir="${examples.dir}"/>

It seems that with the refactoring of the modules-javadocs build system, they were removed and now every module contains only the "src.dir" packageset.

I didn't find a clean way to add another package source to the facet javadocs, besides overriding <javadocs> and inline <invoke-javadoc-module>, to add another packagset.

Is that indeed the best approach, or can <invoke-javadoc-module> be extended to support additional sources?


I don't think we should do this.

Can we merge these examples into the demo/ module?
Otherwise lets make a facet-examples module, if it wants to have its own jar/javadocs/etc.


Reply | Threaded
Open this post in threaded view
|

Re: Facet examples javadoc missing in 4.0?

Robert Muir


On Sat, Nov 24, 2012 at 9:18 AM, Shai Erera <[hidden email]> wrote:
bq. I don't think we should do this.

Why? What's the harm? It's a fairly simple fix to the build.xml and it's not like all other modules have example code under demo, such that the facet one stands out.


It makes the build system, packaging, and release verification more complicated if a single module has multiple artifacts.

I think we should just keep it clean and let a single module be a single module.
Reply | Threaded
Open this post in threaded view
|

Re: Facet examples javadoc missing in 4.0?

Shai Erera

But it is a single module. Yes, it has two artifacts, one of them is example code.

We can discuss if it needs to be resolved or not (I personally don't think it's critical). But the current situation is a bug - the examples javadocs existed and now they're missing.

That's what I want to fix now. I'll open an issue and post a patch.

Separating into a facet-examples module has the same problems as putting it under demo. Maybe we can put it under facet/src/java, but that's not great either...

I suggest that we fix the current bug, and discuss more on LUCENE-3998.

Shai

On Nov 24, 2012 7:22 PM, "Robert Muir" <[hidden email]> wrote:


On Sat, Nov 24, 2012 at 9:18 AM, Shai Erera <[hidden email]> wrote:
bq. I don't think we should do this.

Why? What's the harm? It's a fairly simple fix to the build.xml and it's not like all other modules have example code under demo, such that the facet one stands out.


It makes the build system, packaging, and release verification more complicated if a single module has multiple artifacts.

I think we should just keep it clean and let a single module be a single module.
Reply | Threaded
Open this post in threaded view
|

Re: Facet examples javadoc missing in 4.0?

Robert Muir
In reply to this post by Shai Erera


On Sat, Nov 24, 2012 at 9:18 AM, Shai Erera <[hidden email]> wrote:

We tried that in LUCENE-3998 without success. The problem are facet tests which depend on data that exists in the example package. I don't want to disable tests just for the sake of separation, nor move to the examples package tests that should reside in the module itself.


I don't get this: the demo module does have tests already.

If the demo module includes faceting examples, why wouldnt we move the tests there as well?

In other words, we should have a clean separation of unit testing the code versus testing the examples.
Reply | Threaded
Open this post in threaded view
|

Re: Facet examples javadoc missing in 4.0?

Shai Erera
You're probably right. The thing is, some facet tests rely on the data that exists in the facet examples. The reason is that the examples already contain some large number of documents and facets, so it was easy to test certain features (I think TotalFacetCounts) with large set of documents, without duplicating the information.

Is it necessary, probably not, but that's the current state of affairs. I tend to agree that facet examples should be under demo/. It's just that I'm not willing to lose facet (core) tests, or otherwise, the facet tests would need to depend on demo  ... is it bad?

Really, currently the facet examples build are half broken. The facet/build.xml takes care to generate the facet-examples.jar, compile them and test them. The only piece that was removed, from what I can tell by accident, are the javadocs ...

You know that I always prefer to get to a consensus before opening an issue, but I want to get this fixed. It was my mistake that I didn't notice that when inspecting the 4.0 artifacts .. a mistake which probably demonstrates why it would be better if it were under demo/. But it doesn't mean that until we resolve LUCENE-3998 (if we resolve it), the examples javadocs should be missing from the build.

Shai

On Sat, Nov 24, 2012 at 8:26 PM, Robert Muir <[hidden email]> wrote:


On Sat, Nov 24, 2012 at 9:18 AM, Shai Erera <[hidden email]> wrote:

We tried that in LUCENE-3998 without success. The problem are facet tests which depend on data that exists in the example package. I don't want to disable tests just for the sake of separation, nor move to the examples package tests that should reside in the module itself.


I don't get this: the demo module does have tests already.

If the demo module includes faceting examples, why wouldnt we move the tests there as well?

In other words, we should have a clean separation of unit testing the code versus testing the examples.

Reply | Threaded
Open this post in threaded view
|

Re: Facet examples javadoc missing in 4.0?

Robert Muir


On Sat, Nov 24, 2012 at 10:35 AM, Shai Erera <[hidden email]> wrote:
You're probably right. The thing is, some facet tests rely on the data that exists in the facet examples. The reason is that the examples already contain some large number of documents and facets, so it was easy to test certain features (I think TotalFacetCounts) with large set of documents, without duplicating the information.

Is it necessary, probably not, but that's the current state of affairs. I tend to agree that facet examples should be under demo/. It's just that I'm not willing to lose facet (core) tests, or otherwise, the facet tests would need to depend on demo  ... is it bad?

Really, currently the facet examples build are half broken. The facet/build.xml takes care to generate the facet-examples.jar, compile them and test them. The only piece that was removed, from what I can tell by accident, are the javadocs ...

You know that I always prefer to get to a consensus before opening an issue, but I want to get this fixed. It was my mistake that I didn't notice that when inspecting the 4.0 artifacts .. a mistake which probably demonstrates why it would be better if it were under demo/. But it doesn't mean that until we resolve LUCENE-3998 (if we resolve it), the examples javadocs should be missing from the build.

I think it should be fixed too. But lets do the work to fix it right, otherwise the bug will resurface again and again.
There is no failure in LUCENE-3998: I just did some of the work and stopped when I ran out of time.

Lets fix it correctly and avoid hacks. Otherwise its impossible to know really that the release artifacts for facets are correct.

Reply | Threaded
Open this post in threaded view
|

Re: Facet examples javadoc missing in 4.0?

Shai Erera
It's not that there were failures, but that you moved some tests under demo/ that should belong under facet/. That's the dependency I'm talking about. I tried to resolve it, but it wasn't trivial -- either these tests will depend on demo/, or we rewrite them to use their own data, which will take some time.

It's not really a hack, it worked like that ever since 3.4, only recently removed. If you want, let's mark 3998 as a blocker for 4.1, but for now, this is what I'd like to fix:

Index: lucene/facet/build.xml
===================================================================
--- lucene/facet/build.xml      (revision 1413130)
+++ lucene/facet/build.xml      (working copy)
@@ -75,11 +75,22 @@
   <target name="jar-core" depends="common.jar-core,jar-examples" />

   <target name="javadocs" depends="javadocs-analyzers-common,compile-core">
-    <invoke-module-javadoc>
-      <links>
-        <link href="../analyzers-common"/>
-      </links>
-    </invoke-module-javadoc>
+    <sequential>
+      <mkdir dir="${javadoc.dir}/${name}"/>
+      <invoke-javadoc
+         destdir="${javadoc.dir}/${name}"
+                title="${Name} ${version} ${name} API"
+         linksource="no">
+         <sources>
+           <link href="../core/"/>
+           <link href="../analyzers-common"/>
+           <link href=""/>
+           <packageset dir="${src.dir}"/>
+           <packageset dir="${examples.dir}"/>
+        </sources>
+      </invoke-javadoc>
+      <jarify basedir="${javadoc.dir}/${name}" destfile="${build.dir}/${final.name}-javadoc.jar"/>
+    </sequential>
   </target>

 </project>

It's not the most pretty thing, but it does the work (I verified) and it'll be easy to fix it back if and when we handle 3998. Otherwise I'm afraid that 4.1 will be out without the examples javadocs again.

Shai

On Sat, Nov 24, 2012 at 8:39 PM, Robert Muir <[hidden email]> wrote:


On Sat, Nov 24, 2012 at 10:35 AM, Shai Erera <[hidden email]> wrote:
You're probably right. The thing is, some facet tests rely on the data that exists in the facet examples. The reason is that the examples already contain some large number of documents and facets, so it was easy to test certain features (I think TotalFacetCounts) with large set of documents, without duplicating the information.

Is it necessary, probably not, but that's the current state of affairs. I tend to agree that facet examples should be under demo/. It's just that I'm not willing to lose facet (core) tests, or otherwise, the facet tests would need to depend on demo  ... is it bad?

Really, currently the facet examples build are half broken. The facet/build.xml takes care to generate the facet-examples.jar, compile them and test them. The only piece that was removed, from what I can tell by accident, are the javadocs ...

You know that I always prefer to get to a consensus before opening an issue, but I want to get this fixed. It was my mistake that I didn't notice that when inspecting the 4.0 artifacts .. a mistake which probably demonstrates why it would be better if it were under demo/. But it doesn't mean that until we resolve LUCENE-3998 (if we resolve it), the examples javadocs should be missing from the build.

I think it should be fixed too. But lets do the work to fix it right, otherwise the bug will resurface again and again.
There is no failure in LUCENE-3998: I just did some of the work and stopped when I ran out of time.

Lets fix it correctly and avoid hacks. Otherwise its impossible to know really that the release artifacts for facets are correct.


Reply | Threaded
Open this post in threaded view
|

Re: Facet examples javadoc missing in 4.0?

Robert Muir


On Sat, Nov 24, 2012 at 10:45 AM, Shai Erera <[hidden email]> wrote:
It's not that there were failures, but that you moved some tests under demo/ that should belong under facet/. That's the dependency I'm talking about. I tried to resolve it, but it wasn't trivial -- either these tests will depend on demo/, or we rewrite them to use their own data, which will take some time.

Right, its wrong for these tests to be moved, and thats why i said "there is more work to do here".
These tests should be rewritten to be "unit tests", with a clean separation of the tests that test the example.

Currently its all messy and disorganized. I just didn't have the time to clean this up (it was around release time, I was annoyed at the situation, and took a half-hearted stab at fixing it just in case it was something I could fix in an hour).
 

It's not really a hack, it worked like that ever since 3.4, only recently removed. If you want, let's mark 3998 as a blocker for 4.1, but for now, this is what I'd like to fix:

I don't really want to add hacks here, because who knows what else is broken/will break with packaging. For example, what will the maven -javadocs.jar look like? Is it correct? I have no idea.

Thats why I say I think we should just do the work to fix this correctly.
I don't like adding a special case situation with these examples that will only cause pain in the future.

There is a lot of work to be done with these examples until we release javadocs for them. I don't see any package.htmls, some classes have no description, etc.

Reply | Threaded
Open this post in threaded view
|

Re: Facet examples javadoc missing in 4.0?

Uwe Schindler
The question I have is: why do we need javadocs for the example code? This makes no sense to me!

Uwe



Robert Muir <[hidden email]> schrieb:


On Sat, Nov 24, 2012 at 10:45 AM, Shai Erera <[hidden email]> wrote:
It's not that there were failures, but that you moved some tests under demo/ that should belong under facet/. That's the dependency I'm talking about. I tried to resolve it, but it wasn't trivial -- either these tests will depend on demo/, or we rewrite them to use their own data, which will take some time.

Right, its wrong for these tests to be moved, and thats why i said "there is more work to do here".
These tests should be rewritten to be "unit tests", with a clean separation of the tests that test the example.

Currently its all messy and disorganized. I just didn't have the time to clean this up (it was around release time, I was annoyed at the situation, and took a half-hearted stab at fixing it just in case it was something I could fix in an hour).
 

It's not really a hack, it worked like that ever since 3.4, only recently removed. If you want, let's mark 3998 as a blocker for 4.1, but for now, this is what I'd like to fix:

I don't really want to add hacks here, because who knows what else is broken/will break with packaging. For example, what will the maven -javadocs.jar look like? Is it correct? I have no idea.

Thats why I say I think we should just do the work to fix this correctly.
I don't like adding a special case situation with these examples that will only cause pain in the future.

There is a lot of work to be done with these examples until we release javadocs for them. I don't see any package.htmls, some classes have no description, etc.


--
Uwe Schindler
H.-H.-Meier-Allee 63, 28213 Bremen
http://www.thetaphi.de
Reply | Threaded
Open this post in threaded view
|

Re: Facet examples javadoc missing in 4.0?

Shai Erera
In reply to this post by Robert Muir
I don't hack anything, just return things to how they were. If we make the separation successfully in time for 4.1, then we can change the build.xml back. Otherwise, at least the examples javadocs will be out there.

Since it worked before, I don't think that there will be any issues with maven, unless it didn't work correctly before.

Shai

On Sat, Nov 24, 2012 at 8:53 PM, Robert Muir <[hidden email]> wrote:


On Sat, Nov 24, 2012 at 10:45 AM, Shai Erera <[hidden email]> wrote:
It's not that there were failures, but that you moved some tests under demo/ that should belong under facet/. That's the dependency I'm talking about. I tried to resolve it, but it wasn't trivial -- either these tests will depend on demo/, or we rewrite them to use their own data, which will take some time.

Right, its wrong for these tests to be moved, and thats why i said "there is more work to do here".
These tests should be rewritten to be "unit tests", with a clean separation of the tests that test the example.

Currently its all messy and disorganized. I just didn't have the time to clean this up (it was around release time, I was annoyed at the situation, and took a half-hearted stab at fixing it just in case it was something I could fix in an hour).
 

It's not really a hack, it worked like that ever since 3.4, only recently removed. If you want, let's mark 3998 as a blocker for 4.1, but for now, this is what I'd like to fix:

I don't really want to add hacks here, because who knows what else is broken/will break with packaging. For example, what will the maven -javadocs.jar look like? Is it correct? I have no idea.

Thats why I say I think we should just do the work to fix this correctly.
I don't like adding a special case situation with these examples that will only cause pain in the future.

There is a lot of work to be done with these examples until we release javadocs for them. I don't see any package.htmls, some classes have no description, etc.


Reply | Threaded
Open this post in threaded view
|

Re: Facet examples javadoc missing in 4.0?

Robert Muir


On Sat, Nov 24, 2012 at 11:05 AM, Shai Erera <[hidden email]> wrote:
I don't hack anything, just return things to how they were. If we make the separation successfully in time for 4.1, then we can change the build.xml back. Otherwise, at least the examples javadocs will be out there.

Since it worked before, I don't think that there will be any issues with maven, unless it didn't work correctly before.


I don't really understand this. Again I reiterate: we really shouldn't do this hack.
I want to know whats in our artifacts: e.g. if facet module contains "A+B" on apache.org/dist but only "A" on maven and so on, this is really confusing and messy.

I dont think faceting needs any special case here. We just need to clean up the mess.
Reply | Threaded
Open this post in threaded view
|

Re: Facet examples javadoc missing in 4.0?

Shai Erera
Uwe, it's a good question :). The javadocs aren't that rich, and the lucene-facet-examples.jar is created with the source code anyway ...

Robert, I don't know maven enough but I do see this in maven/lucene/facet/pom.xml.template:

            <configuration>
              <sources>
                <source>${module-path}/src/examples</source>
              </sources>
            </configuration>

Does it mean that it's ok?

Per Uwe's (enlightening) question, I'm willing to let go of this for now.

Shai

On Sat, Nov 24, 2012 at 9:08 PM, Robert Muir <[hidden email]> wrote:


On Sat, Nov 24, 2012 at 11:05 AM, Shai Erera <[hidden email]> wrote:
I don't hack anything, just return things to how they were. If we make the separation successfully in time for 4.1, then we can change the build.xml back. Otherwise, at least the examples javadocs will be out there.

Since it worked before, I don't think that there will be any issues with maven, unless it didn't work correctly before.


I don't really understand this. Again I reiterate: we really shouldn't do this hack.
I want to know whats in our artifacts: e.g. if facet module contains "A+B" on apache.org/dist but only "A" on maven and so on, this is really confusing and messy.

I dont think faceting needs any special case here. We just need to clean up the mess.

Reply | Threaded
Open this post in threaded view
|

Re: Facet examples javadoc missing in 4.0?

Robert Muir


On Sat, Nov 24, 2012 at 11:18 AM, Shai Erera <[hidden email]> wrote:
Uwe, it's a good question :). The javadocs aren't that rich, and the lucene-facet-examples.jar is created with the source code anyway ...

Robert, I don't know maven enough but I do see this in maven/lucene/facet/pom.xml.template:

            <configuration>
              <sources>
                <source>${module-path}/src/examples</source>
              </sources>
            </configuration>

Does it mean that it's ok?

That may be ok in this particular example, but having this all in src/examples causes all kinds of questions.

Do these files all contain apache license headers? I have no idea: because it has source code in a special place probably ignored by rat tasks.
Will the examples work on any system regardless of its locale? I have no idea: its source is in a special place probably ignored by forbidden-api checker.

And the list goes on and on.

These are the reasons why I proposed a long time ago to remove the special case of lucene/src/demo and just make it an "ordinary module" with respect to the build system: otherwise it causes lots of bugs which always happened at release time.

Finally for examples, it makes sense for them to be in the demo module anyway, as its configured in a special way to -linksource the examples into its javadocs, so that we can freely link to the precise versioned source code in that release (rather than things like svn locations, etc).

Reply | Threaded
Open this post in threaded view
|

Re: Facet examples javadoc missing in 4.0?

Shai Erera
I'm not arguing that it shouldn't be improved. It should. I'll try to take a look at the tests again.

Shai

On Sat, Nov 24, 2012 at 9:24 PM, Robert Muir <[hidden email]> wrote:


On Sat, Nov 24, 2012 at 11:18 AM, Shai Erera <[hidden email]> wrote:
Uwe, it's a good question :). The javadocs aren't that rich, and the lucene-facet-examples.jar is created with the source code anyway ...

Robert, I don't know maven enough but I do see this in maven/lucene/facet/pom.xml.template:

            <configuration>
              <sources>
                <source>${module-path}/src/examples</source>
              </sources>
            </configuration>

Does it mean that it's ok?

That may be ok in this particular example, but having this all in src/examples causes all kinds of questions.

Do these files all contain apache license headers? I have no idea: because it has source code in a special place probably ignored by rat tasks.
Will the examples work on any system regardless of its locale? I have no idea: its source is in a special place probably ignored by forbidden-api checker.

And the list goes on and on.

These are the reasons why I proposed a long time ago to remove the special case of lucene/src/demo and just make it an "ordinary module" with respect to the build system: otherwise it causes lots of bugs which always happened at release time.

Finally for examples, it makes sense for them to be in the demo module anyway, as its configured in a special way to -linksource the examples into its javadocs, so that we can freely link to the precise versioned source code in that release (rather than things like svn locations, etc).


Reply | Threaded
Open this post in threaded view
|

Re: Facet examples javadoc missing in 4.0?

Robert Muir
I can help too, I don't mean to be just a complainer. Maybe its only a little more work.

Like i said, that first "patch" i threw up on the issue was just a quick stab.

Instead of spending a lot of time analyzing what could/couldn't work I just took a quick stab in the chance that it would be simple (sometimes things like this work out), but it didn't: there is some more to do. I wish i had had more time to see it through back then before 4.0...

On Sat, Nov 24, 2012 at 11:35 AM, Shai Erera <[hidden email]> wrote:
I'm not arguing that it shouldn't be improved. It should. I'll try to take a look at the tests again.

Shai


On Sat, Nov 24, 2012 at 9:24 PM, Robert Muir <[hidden email]> wrote:


On Sat, Nov 24, 2012 at 11:18 AM, Shai Erera <[hidden email]> wrote:
Uwe, it's a good question :). The javadocs aren't that rich, and the lucene-facet-examples.jar is created with the source code anyway ...

Robert, I don't know maven enough but I do see this in maven/lucene/facet/pom.xml.template:

            <configuration>
              <sources>
                <source>${module-path}/src/examples</source>
              </sources>
            </configuration>

Does it mean that it's ok?

That may be ok in this particular example, but having this all in src/examples causes all kinds of questions.

Do these files all contain apache license headers? I have no idea: because it has source code in a special place probably ignored by rat tasks.
Will the examples work on any system regardless of its locale? I have no idea: its source is in a special place probably ignored by forbidden-api checker.

And the list goes on and on.

These are the reasons why I proposed a long time ago to remove the special case of lucene/src/demo and just make it an "ordinary module" with respect to the build system: otherwise it causes lots of bugs which always happened at release time.

Finally for examples, it makes sense for them to be in the demo module anyway, as its configured in a special way to -linksource the examples into its javadocs, so that we can freely link to the precise versioned source code in that release (rather than things like svn locations, etc).