accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ke...@deenlo.com
Subject Re: Review Request 30236: Reorganize README
Date Fri, 30 Jan 2015 23:21:58 GMT


> On Jan. 27, 2015, 8:04 p.m., Christopher Tubbs wrote:
> > TESTING.md, lines 20-53
> > <https://reviews.apache.org/r/30236/diff/5/?file=836999#file836999line20>
> >
> >     This whole section feels like a maven tutorial. It might be sufficient to mention
that unit tests are executed by the maven-surefire-plugin, and integration tests are executed
by the maven-failsafe-plugin, and a link to the "Introduction to the Maven Build Lifecycle"
site, and each of those plugins for more information.
> >     
> >     Some of the other non-maven content, like the minimal requirements, and the
run-length seem appropriate and relevant, but there's a lot of detail here about how standard
maven plugins are working, and there are better resources for that information than our particular
execution of them.
> 
> kturner wrote:
>     oh no more comments about TESTING.md.  All I did in this patch was rename it from
TESTING to TESTING.md :)  However I am ok with reviewing and improving it here, its just that
all of the comments about it were completely unexpected.
>     
>     I don't feel like the following long sentence is equivalent to a maven tutorial.
 I can work the links in, but I see no harm in leaving it.
>     
>         Integration tests can be run by invoking `mvn integration-test` at the top of
the Apache Accumulo source tree; however,
>         like `mvn package` being recommended for unit tests, `mvn verify` is often the
recommended avenue to run the integration tests.
>     
>     Do you want to just remove the following?
>     
>         Take note that when invoking the `integration-test` lifecycle phase, other functions
will also be enabled which include
>         static analysis (findbugs) and software license checks (release analysis tool
-- RAT).
> 
> Christopher Tubbs wrote:
>     I just think it's unusual to describe how to execute each individual phase in the
maven build lifecycle. That's what I mean by "maven tutorial". The build instructions describe
`mvn package`. Here, it should just describe `mvn verify`. After all, the integration tests
get executed during the integration-test phase of the build lifecycle, but they don't get
checked for errors until the verify phase anyway. So, integration testing is not actually
completed unless you execute `mvn verify`. So, some of this is technically correct, but misleading,
and doesn't do justice the actual behavior of these goals.
>     
>     > Do you want to just remove the following?
>     
>     That would help, but I also think it would be better to simply describe `mvn verify`
instead of the other goals.

Changes in patch #7 only mention `mvn verify`


- kturner


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30236/#review69865
-----------------------------------------------------------


On Jan. 30, 2015, 11:20 p.m., kturner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30236/
> -----------------------------------------------------------
> 
> (Updated Jan. 30, 2015, 11:20 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-1515
>     https://issues.apache.org/jira/browse/ACCUMULO-1515
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Reorganized information in README and converted to markdown.  
> 
> At this point I like the INSTALL.md document, but do not really like the content of the
README.md ATM.  Putting this up for review to get suggestions.
> 
> See how the markdown looks on GH : https://github.com/keith-turner/accumulo/tree/ACCUMULO-1515
> 
> 
> Diffs
> -----
> 
>   BUILD.md PRE-CREATION 
>   INSTALL.md PRE-CREATION 
>   NOTICE af212c2 
>   README 4ebb078 
>   README.md PRE-CREATION 
>   TESTING cf2afba 
>   TESTING.md PRE-CREATION 
>   assemble/src/main/assemblies/component.xml 3f18da3 
> 
> Diff: https://reviews.apache.org/r/30236/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> kturner
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message