Return-Path: 
 <issues-return-50732-apmail-hbase-issues-archive=hbase.apache.org@hbase.apache.org>
X-Original-To: apmail-hbase-issues-archive@www.apache.org
Delivered-To: apmail-hbase-issues-archive@www.apache.org
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by minotaur.apache.org (Postfix) with SMTP id AA687CF97
	for <apmail-hbase-issues-archive@www.apache.org>;
 Sat,  2 Jun 2012 18:22:23 +0000 (UTC)
Received: (qmail 90851 invoked by uid 500); 2 Jun 2012 18:22:23 -0000
Delivered-To: apmail-hbase-issues-archive@hbase.apache.org
Received: (qmail 90806 invoked by uid 500); 2 Jun 2012 18:22:23 -0000
Mailing-List: contact issues-help@hbase.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:issues-help@hbase.apache.org>
List-Unsubscribe: <mailto:issues-unsubscribe@hbase.apache.org>
List-Post: <mailto:issues@hbase.apache.org>
List-Id: <issues.hbase.apache.org>
Delivered-To: mailing list issues@hbase.apache.org
Received: (qmail 90795 invoked by uid 99); 2 Jun 2012 18:22:23 -0000
Received: from issues-vm.apache.org (HELO issues-vm) (140.211.11.160)
    by apache.org (qpsmtpd/0.29) with ESMTP; Sat, 02 Jun 2012 18:22:23 +0000
Received: from isssues-vm.apache.org (localhost [127.0.0.1])
	by issues-vm (Postfix) with ESMTP id 4343414285A
	for <issues@hbase.apache.org>; Sat,  2 Jun 2012 18:22:23 +0000 (UTC)
Date: Sat, 2 Jun 2012 18:22:23 +0000 (UTC)
From: "Jesse Yates (JIRA)" <jira@apache.org>
To: issues@hbase.apache.org
Message-ID: <1146360393.30754.1338661343277.JavaMail.jiratomcat@issues-vm>
In-Reply-To: <479406675.25247.1338527182931.JavaMail.jiratomcat@issues-vm>
Subject: [jira] [Commented] (HBASE-6145) Fix site target post modularization
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
X-JIRA-FingerPrint: 30527f35849b9dde25b450d4833f0394


    [ https://issues.apache.org/jira/browse/HBASE-6145?page=3Dcom.atlassian=
.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=3D1328=
7995#comment-13287995 ]=20

Jesse Yates commented on HBASE-6145:
------------------------------------

A pretty in depth analysis, hopefully not too much:
Patch didn't apply cleanly with git, but got it go with basic patch command=
 (mostly - looks like you might be a little behind in the docbkx?).
I'm just going to step through per pom=E2=80=A6

hbase-server/pom.xml
{quote}
+          <version>${avro.version}</version>
{quote}

This (and the rest of the dependency info) should be in the parent pom's de=
pendencyManagement section. Just as general style, if another module wants =
to use avro, they should just declare the dependency, and not worry about m=
aking sure they have the right version, excludes, etc (I know we are droppi=
ng avro soon, but we should still do the right thing).=20

Also, looks like your spacing is off for the added dependencies - 2 spaces,=
 not tabs in xml.

hbase-common/pom.xml
{quote}
+    <plugins>
+      <plugin>
+        <artifactId>maven-surefire-plugin</artifactId>
{quote}

This can/should stay in the pluginManagement section. Surefire is part of t=
he default maven things to run, so it will just pick up the configuration/e=
xecutions from the management section - this also keeps all the surefire st=
uff in the same sections in all poms.

Also, does anything actually happen if we remove:
{quote}
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-site-plugin</artifactId>
{quote}

from this pom? It may create the target/site (side effect of site being a c=
ore part of maven, so all modules can respond to it), but is any actual wor=
k done?

Okay, onto the parent pom (/hbase/pom.xml):

Why the removal of the hbase-assembly module? In the official docs, it actu=
ally says to use an assembly module. This is particularly poignant because =
the alternative is to use the assembly:assembly descriptor, which is deprec=
ated=E2=80=A6 The docs say that you can use assembly:assembly from within t=
he parent pom (but doesn't say what that actually means) - any way to tie t=
he assembly:single phase to call the assembly:single/:assembly phase in the=
 children poms?

In src/assembly/all.xml, this comment is no longer applicable.=20
{quote}
    <!-- This is only necessary until maven fixes the intra-project depende=
ncy bug=20
      in maven 3.0. Until then, we have to include the test jars for sub-pr=
ojects. When=20
      fixed, the below dependencySet stuff is sufficient for pulling in the=
 test jars as=20
      well, as long as they are added as dependencies in this project. Righ=
t now, we only=20
      have 1 submodule to accumulate, but we can copy/paste as necessary un=
til maven is=20
      fixed. -->
{quote}

Also, it would be awesome to move file set matching to a more general regex=
, rather than tying it to the maven property (which is defined in the main =
pom).=20

General nit: I prefer having the properties above the build, since the prop=
erties are used in the build section, but that's just style.

{quote}
<!--Pass -DskipJavadoc=3Dtrue on command-line to skip javadoc building-->
{quote}
when building the site? Also, can't you just pass in -DskipJavadoc?

{quote}
<version>${maven.assembly.version}</version>
{quote}
and=20
{quote}
<version>${maven.site.version}</version>
{quote}
Would be nice to add:
{code}
<!--$NO-MVN-MAN-VER$-->
{code}
to the end of the lines to remove the eclipse warning

{quote}
        <plugin>
          <groupId>org.codehaus.mojo</groupId>
          <artifactId>xml-maven-plugin</artifactId>
{quote}
configuration formatting is off.

{quote}
            <execution>
              <id>copy-docbkx</id>
              <goals>
                <goal>copy-resources</goal>
              </goals>
              <phase>pre-site</phase>
              <configuration>
                <outputDirectory>target/site</outputDirectory>
                <resources>
                  <resource>
                    <directory>${basedir}/target/docbkx</directory>
                    <includes>
                      <include>**/**</include>
                    </includes>
                  </resource>
                </resources>
              </configuration>
            </execution>
          </executions>
          <configuration>
            <escapeString>\</escapeString>
          </configuration>
{quote}
Is the escape string new here? What property are you avoiding overriding? A=
lso, why are you moving them in the first place? You can just set the targe=
t directory to be ${basedir}/target/docbkx in the docbkx plugin:
{code}
         <groupId>com.agilejava.docbkx</groupId>
          <artifactId>docbkx-maven-plugin</artifactId>
{code}

Here, you can also move the common traits to the 'top level' configuration =
for the plugin, then just put the differences in each execution. For exampl=
e:
{code}
        <plugin>
          <groupId>com.agilejava.docbkx</groupId>
          <artifactId>docbkx-maven-plugin</artifactId>
          <version>2.0.14</version>
          <inherited>false</inherited>
          <dependencies>
=09=09=09<!--=E2=80=A6 some stuff -->
=09=09</dependencies>
=09=09<configuration>
=09=09=09<!--common configuration stuff here -->
=09=09</configuration>
          <executions>
            <execution>
              <id>multipage</id>
              <goals>
                <goal>generate-html</goal>
              </goals>
              <phase>pre-site</phase>
              <configuration>
=09=09=09<!-- multi-page configuration stuff here -->
=09=09=09=E2=80=A6
{code}

This also seems excessive:
{quote}
        <plugin>
          <artifactId>maven-resources-plugin</artifactId>
          <version>${maven.resources.plugin.version}</version>
          <inherited>false</inherited>
          <executions>
            <execution>
              <id>copy-javadocs</id>
              <goals>
                <goal>copy-resources</goal>
              </goals>
              <phase>pre-site</phase>
{quote}
With the aggregate goal, you can just have all the javadocs copied into the=
 right directory in this module when you build them; at least that is what =
is was doing before.

{quote}
          <artifactId>maven-assembly-plugin</artifactId>
          <version>${maven.assembly.version}</version>
          <inherited>false</inherited>
        <configuration>
{quote}
Whitespace is messed up for the configuration section (and actually the who=
le section - spaces, not tabs!)

It would be sweet if we can get the eclipse natures/inclusions fixed for=20
{quote}
        <plugin>
          <groupId>org.apache.maven.plugins</groupId>
          <artifactId>maven-eclipse-plugin</artifactId>
          <version>2.8</version>
        </plugin>
{quote}
This way the hbase top level folder wouldn't have a java nature by default.=
 Also, for instance, we can add the assembly, docbkx, etc stuff for /hbase =
as src folders, and for hbase-server finally fix having to fix the classpat=
h when you load it into eclipse (or do eclipse:eclipse). See:
http://maven.apache.org/plugins/maven-eclipse-plugin/examples/specifying-so=
urce-path-inclusions-and-exclusions.html The latter might be a little out o=
f scope for this ticket, but if you have the time...

Other than that, looks good to me (didn't try building/testing yet). Good s=
tuff stack.
               =20
> Fix site target post modularization
> -----------------------------------
>
>                 Key: HBASE-6145
>                 URL: https://issues.apache.org/jira/browse/HBASE-6145
>             Project: HBase
>          Issue Type: Task
>            Reporter: stack
>            Assignee: stack
>         Attachments: site.txt, site2.txt, sitev3.txt
>
>


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrato=
rs: https://issues.apache.org/jira/secure/ContactAdministrators!default.jsp=
a
For more information on JIRA, see: http://www.atlassian.com/software/jira