crunch-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthias Friedrich" <>
Subject Re: Review Request: Add PGroupedTable.combineValues(Aggregator) and Aggregators.
Date Sun, 11 Nov 2012 14:10:52 GMT

This is an automatically generated e-mail. To reply, visit:

(Updated Nov. 11, 2012, 2:10 p.m.)

Review request for crunch.


Updated based on Josh's review, deprecated most of CombineFn and added Javadoc to Aggregators.
As far as I'm concerned, this is pretty much ready to go.

One open issue: Have a look at
- Due to a bug in the javadoc tool (open since 2006) and the way it displays inner classes,
we get "CombineFn.Aggregator" everywhere. Should we make Aggregator a top-level type? This
would clean up documentation quite a bit.


Here's a patch to test ReviewBoard and to give you an impression of the new Aggregators class
and the changes this would require. I have simplified things even further than I originally
planned, so I need some feedback if I'm on the right track here.

Even when approved, I will not commit this immediately. There are a few things that need to
be done that I'll discuss on crunch-dev.

Diffs (updated)

  crunch-examples/src/main/java/org/apache/crunch/examples/ 8abbb73 
  crunch-examples/src/main/java/org/apache/crunch/examples/ 44776ea 
  crunch-examples/src/main/java/org/apache/crunch/examples/ 691721d

  crunch/src/it/java/org/apache/crunch/ 0d5803e 
  crunch/src/it/java/org/apache/crunch/ 5124663 
  crunch/src/it/java/org/apache/crunch/fn/ PRE-CREATION 
  crunch/src/it/java/org/apache/crunch/lib/ b6f5029 
  crunch/src/it/java/org/apache/crunch/test/ PRE-CREATION 
  crunch/src/it/resources/org/apache/crunch/fn/AggregatorsITData/ints.txt PRE-CREATION 
  crunch/src/main/java/org/apache/crunch/ d45940b 
  crunch/src/main/java/org/apache/crunch/ e727b70 
  crunch/src/main/java/org/apache/crunch/fn/ PRE-CREATION 
  crunch/src/main/java/org/apache/crunch/impl/mem/collect/ 0ee4c3f 
  crunch/src/main/java/org/apache/crunch/impl/mr/collect/ fee381d 
  crunch/src/main/java/org/apache/crunch/lib/ f28cca4 
  crunch/src/test/java/org/apache/crunch/fn/ PRE-CREATION 
  pom.xml 84d481f 



Integration test works, as did a simple example on a single node cluster. Added an integration
test and unit tests based on CombineFnTest.


Matthias Friedrich

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