hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sean Busbey (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-4593) Design and document the official procedure for posting patches, commits, commit messages, etc. to smooth process and make integration with tools easier
Date Thu, 07 Aug 2014 14:40:13 GMT

    [ https://issues.apache.org/jira/browse/HBASE-4593?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14089273#comment-14089273
] 

Sean Busbey commented on HBASE-4593:
------------------------------------

There are a few places in the rendered PDF where it looks like spacing is missing between
plain text and formatted (e.g. links, monospace). It looks like there are spaces in the source
patch though, so probably a rendering artifact?

What's the difference between section 18.1 and 18.10? they seem to be aimed at the same thing.
If they're not, 18.1 should have a pointer to 18.10.

{quote}
+        <para> If you are looking to contribute to Apache HBase, look for issues in
JIRA tagged with
+            the label 'beginner': <link
+                xlink:href="https://issues.apache.org/jira/issues/?jql=project%20%3D%20HBASE%20AND%20labels%20in%20(beginner)"
+                >project = HBASE AND labels in (beginner)</link>. These are issues
HBase

{quote}

personally, I think using "issues in JIRA tagged with the label 'beginner'" as teh link text
(instead of the jira query text) makes the section read clearer.

{quote}
+                <link xlink:href="http://git.apache.org/">Apache Git</link> page.
</para>
{quote}

nit: trailing whitespace between '.' and end of paragraph.


{quote}
+        <section>
+            <title>Other IDEs</title>
+            <para>TODO - Please contribute</para>
         </section>
{quote}

Can we make this more specific or leave it out? Ideally a follow-on umbrella jira that we
can reference here. for subtasks of that umbrella, I know IntelliJ is popular with the IDE
users I talk to.

{quote}
+                    <code>compile-protobuf</code> to do this.</para>
+            <programlisting language="bourne">mvn compile -Dcompile-protobuf</programlisting>
+            <programlisting language="bourne">mvn compile -Pcompile-protobuf</programlisting>
{quote}

This looks like I need to run both of these commands to rebuild the protobufs. I believe I
only need to run one of them. We should pick which one is preferred and only suggest that
one.

The protoc.path example looks like we prefer the "-Dcompile-protobuf" version. I'm pretty
sure the idiomatic way for maven is the profile, so perhaps we should make both use that?

{quote}
+        <section xml:id="build.snappy">
+            <title>Building in snappy compression support</title>
+            <para>Pass <code>-Dsnappy</code> to trigger the <code>snappy</code>
maven profile for
+                building Google Snappy native libraries into HBase. See also <xref
+                    linkend="snappy.compression"/></para>
{quote}

Similar to the protobuf bit, can we change this to use the profile directly?

{quote}
+            <para>HBase 0.96.x will run on Hadoop 1.x or Hadoop 2.x. HBase 0.98 still
runs on both,
+                but HBase 0.98 deprecates use of Hadoop 1. HBase 1.x will <emphasis>not</emphasis>
+                run on Hadoop 1. In the following procedures, we make a distinction between
HBase
+                1.x builds and the awkward process involved building HBase 0.96/0.98 for
either
+                Hadoop 1 or Hadoop 2 targets. </para>
{quote}

Maybe end with a link to the java ref for more info?


{quote}
+            <formalpara>
+                <title>Maven Version</title>
+                <para>You must use maven 3.0.x (Check by running <command>mvn
-version</command>).
                 </para>
+            </formalpara>
{quote}

Is this generally true? Should we add it to the section on basic compilation? Seems more likely
to trip up a new person trying to build than someone creating a release candidate.

{quote}
+                <title>Before You Begin</title>
+                <para>Before you make a release candidate, do a practise run by deploying
a
{quote}

Do we have a documentation style guide that covers British v American english usage?

{quote}
+                    intervention is needed here), the checking of the produced artifacts
to ensure
+                    they are 'good' -- e.g. undoing the produced tarballs, eyeballing them
to make
+                    sure they look right then starting and checking all is running properly
-- and
{quote}

"unpacking" would be clearer than "undoing"

{quote}
+                    <title>If you used the <filename>make_rc.sh</filename>
script instead of doing
+                        the above manually,, do your sanity checks now.</title>
{quote}

nit: extraneous comma.

{quote}
+                docbkx:generate-html</command> (TODO: It looks like you have to run
<command>mvn
+                site</command> first because docbkx wants to include a transformed
+                <filename>hbase-default.xml</filename>. Fix). When you run mvn
site, we do the
{quote}

Can we make a jira for this and then reference it here?

{quote}
+        <section xml:id="hbase.rc.voting">
+            <title>Voting on Release Candidates</title>
+            <para> Everyone is encouraged to try and vote on HBase release candidates.
Only the
+                votes of PMC members are binding. PMC members, please read this WIP doc on
policy
+                voting for a release candidate, <link
+                    xlink:href="https://github.com/rectang/asfrelease/blob/master/release.md"
+                    >Release Policy</link>. <quote>Before casting +1 binding
votes, individuals are
+                    required to download the signed source code package onto their own hardware,
+                    compile it as provided, and test the resulting executable on their own
platform,
+                    along with also validating cryptographic signatures and verifying that
the
+                    package meets the requirements of the ASF policy on releases.</quote>
Regards
+                the latter, run <command>mvn apache-rat:check</command> to verify
all files are
+                suitably licensed. See <link xlink:href="http://search-hadoop.com/m/DHED4dhFaU"
+                    >HBase, mail # dev - On recent discussion clarifying ASF release policy</link>.
+                for how we arrived at this process. </para>
+        </section>
{quote}

I don't think this should be in section 18.7. Maybe it's meant to be it's own subsection under
18? Or maybe under 18.5?

{quote}
+                <para> Since HBase version 1.0.0 (<link
+                        xlink:href="https://issues.apache.org/jira/browse/HBASE-11348">HBASE-11348
+                        Make frequency and sleep times of chaos monkeys configurable</link>),
the
{quote}

Do we have a documentation style guide for referencing jiras? This instance uses "JIRA-NUM
title of jira" and elsewhere we use "JIRA-NUM". Either is fine, but we should be consistent.

{quote}
+            <title>Jira</title>
+            <para>Check for existing issues in <link
+                    xlink:href="https://issues.apache.org/jira/browse/HBASE">Jira</link>.
If it's
+                either a new feature request, enhancement, or a bug, file a ticket. </para>
{quote}

Add a link to beginner jiras, similar to the one in section 18.1. 

{quote}
             <para>The following information is from <link
-                    xlink:href="http://blog.cloudera.com/blog/2013/09/how-to-test-hbase-applications-using-popular-tools/">http://blog.cloudera.com/blog/2013/09/how-to-test-hbase-applications-using-popular-tools/</link>.
-                The following sections discuss JUnit, Mockito, MRUnit, and HBaseTestingUtility.
</para>
+                    xlink:href="http://blog.cloudera.com/blog/2013/09/how-to-test-hbase-applications-using-popular-tools/"
+                    >http://blog.cloudera.com/blog/2013/09/how-to-test-hbase-applications-using-popular-tools/</link>.
+                The following sections discuss JUnit, Mockito, MRUnit, and HBaseTestingUtility.
{quote}

suggest changing to something like "The following sections discuss JUnit, Mockito, MRUnit,
and HBaseTestingUtility. The information comes from [a community blog post about testing HBase
applications|http://blog.cloudera.com/blog/2013/09/how-to-test-hbase-applications-using-popular-tools/]"

----

18.11.2 appears to be about writing tests for applications written on top of HBase. The rest
of 18.11 is about developing the HBase codebase. Probably they need to be broken apart?

----
{quote}
+            <para>See the aforementioned Apache Commons link for how to make patches
against a
+                checked out subversion repository.</para>
{quote}

This seems wrong. Should it say "a checked out git repository" ?

{quote}
+                <listitem>
+                    <para>Submit one single patch for a fix. If necessary, use <code>git
+                            squash</code> to merge local commits into a single one
first.</para>
+                </listitem>
{quote}

I don't think git squash is what we want here. Instead say "If necessary, squash local commits
into a single one first" with a link to a guide on squashing

* [interactive rebase|http://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits]
* [squash merge from feature branch|http://365git.tumblr.com/post/4364212086/git-merge-squash]
(or maybe [a SO question that gives less detail|http://stackoverflow.com/questions/5308816/how-to-use-git-merge-squash])

({{git squash}} isn't part of the normal git distro. the closest I could find from a third
party automated the {{git merge --squash && git commit}} workflow)

{quote}
+                <listitem xml:id="submitting.patches.naming">
+                    <para>The patch should have the JIRA ID in the name. If you generating
from a
+                        branch, include the target branch in the filename. A common naming
scheme
+                        for patches is:</para>
+                    <screen>HBASE-<replaceable>XXXX</replaceable>.patch</screen>
+                    <screen>HBASE-<replaceable>XXXX</replaceable>-1.patch</screen>
+                    <screen>HBASE-<replaceable>XXXX</replaceable>-0.90.patch</screen>
+                </listitem>
{quote}

The previous guidance was to use an "_" instead of "-" between the Jira project and the Jira
number (i.e. HBASE_XXXX rather than HBASE-XXXX). I actually prefer the dash, but want to make
sure we're making a conscientious change.

If these examples are just meant to show branch changes, then the middle one should be HBASE-XXXX-branch-1.patch
because the branch name is "branch-1".


{quote}
+                        patch for the whole fix), using the <menuchoice>
+                            <guimenu>More</guimenu>
+                            <guimenuitem>Attach Patch</guimenuitem>
+                        </menuchoice> dialog. Next, click the <guibutton>Patch
Available</guibutton>
{quote}

The menu item is "Attach Files" not "Attach Patch".


{quote}

+                <listitem>
+                    <para>If you need to revise your patch, leave the previous patch
file(s)
+                        attached to the JIRA, and upload the new one with a revision ID.
Cancel the
+                        Patch Available flag and then re-trigger it, by toggling the
+                            <guibutton>Patch Available</guibutton> button in
JIRA.</para>
+                </listitem>
{quote}

Since the "submitting a patch again" section is gone, we don't have any info on what "with
a revision ID" means. Can we add an example? Preferably one that is for a patch against master
and one that's against a branch.

{quote}
+                patches from new submitters.</para>
+            <para> See the <link
+                    xlink:href="http://www.oracle.com/technetwork/java/codeconv-138413.html">Java
+                    coding standards</link> for more information on coding conventions
in Java.
+            </para>
{quote}

I don't know if we want to keep referencing the Java coding standards (it hasn't been updated
since 1999). If we do, the correct link is now http://www.oracle.com/technetwork/java/index-135089.html
and the formal name is actually the Code Conventions for the Java Programming Language.


-----

We still have a section on "Submitting incremental patches." Can we remove this? I believe
we want to encourage contributors to attach just a single patch for a given jira, and to incorporate
feedback into another single patch. If something is really big enough to need breaking into
multiple patches, probably it should be against multiple sub-task jiras.

> Design and document the official procedure for posting patches, commits, commit messages,
etc. to smooth process and make integration with tools easier
> -------------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HBASE-4593
>                 URL: https://issues.apache.org/jira/browse/HBASE-4593
>             Project: HBase
>          Issue Type: Task
>          Components: documentation
>            Reporter: Jonathan Gray
>            Assignee: Misty Stanley-Jones
>         Attachments: HBASE-4593.patch, HBASE-4593.pdf
>
>
> I have been building a tool (currently called reposync) to help me keep the internal
FB hbase-92-based branch up-to-date with the public branches.
> Various inconsistencies in our process has made it difficult to automate a lot of this
stuff.
> I'd like to work with everyone to come up with the official best practices and stick
to it.
> I welcome all suggestions.  Among some of the things I'd like to nail down:
> - Commit message format
> - Best practice and commit message format for multiple commits
> - Multiple commits per jira vs. jira per commit, what are the exceptions and when
> - Affects vs. Fix versions
> - Potential usage of [tags] in commit messages for things like book, scripts, shell...
maybe even whatever is in the components field?
> - Increased usage of JIRA tags or labels to mark exactly which repos a JIRA has been
committed to (potentially even internal repos?  ways for a tool to keep track in JIRA?)
> We also need to be more strict about some things if we want to follow Apache guidelines.
 For example, all final versions of a patch must be attached to JIRA so that the author properly
assigns it to Apache.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Mime
View raw message