Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

Split the scalding platform out into packages #434

Merged
merged 7 commits into from
Feb 6, 2014

Conversation

ianoc
Copy link
Collaborator

@ianoc ianoc commented Feb 6, 2014

Make it easier to iterate on the scalding platform by separating more dense files the single folder out into its components to see what should live where

@@ -27,12 +28,12 @@ import Conversion.asMethod
/** Services and Stores are very similar, but not exact.
* This shares the logic for them.
*/
class BatchedOperations(batcher: Batcher) {
private[scalding] class BatchedOperations(batcher: Batcher) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why? These look pretty useful for Tsar or others doing operations with batches?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't see any uses outside of the scalding platform for it currently. I'd rather be defensive about whats public and not so we know what's safe to change and not? If spark uses it then it might move again, so we don't need more type aliases?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to keep it public i'll add the type alias in scalding and remove it ?

@johnynek
Copy link
Collaborator

johnynek commented Feb 6, 2014

If we remove the private, I mentioned above, and this goes green, I think it is good to merge.

johnynek added a commit that referenced this pull request Feb 6, 2014
…form

Split the scalding platform out into packages
@johnynek johnynek merged commit 92ebae0 into develop Feb 6, 2014
@johnynek johnynek deleted the feature/modularize_scalding_platform branch February 6, 2014 22:25
snoble pushed a commit to snoble/summingbird that referenced this pull request Sep 8, 2017
Moments returns NaN when count is too low for higher order statistics
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants