Skip to content

standardize naming of 'no-op' aggregators#6960

Merged
jihoonson merged 1 commit intoapache:masterfrom
clintropolis:consistent-noop-agg-naming
Apr 2, 2019
Merged

standardize naming of 'no-op' aggregators#6960
jihoonson merged 1 commit intoapache:masterfrom
clintropolis:consistent-noop-agg-naming

Conversation

@clintropolis
Copy link
Copy Markdown
Member

Resolves #6934 by prefixing all 'no-op' aggregators with Noop to follow convention of built-in NoopAggregator and NoopBufferAggregator

@clintropolis
Copy link
Copy Markdown
Member Author

Since I'm renaming a few files in the area already, any upvotes for renaming some of the other datasketches extension post-aggregator types to make navigating to the right file easier? I would like to switch around what is currently the middle of the compound name to the front, like ArrayOfDoublesSketchToNumEntriesPostAggregator -> NumEntriesArrayOfDoublesSketchPostAggregator

screen shot 2019-01-30 at 1 17 41 am

(Open to other ideas too)

@jihoonson
Copy link
Copy Markdown
Contributor

jihoonson commented Apr 1, 2019

Should EmptySequence and EmptyIterator also be renamed?

Since I'm renaming a few files in the area already, any upvotes for renaming some of the other datasketches extension post-aggregator types to make navigating to the right file easier?

I think it depends on how many classes you are thinking to rename. I'm fine by doing it in this PR if it's small enough.

@clintropolis
Copy link
Copy Markdown
Member Author

Should EmptySequence and EmptyIterator also be renamed?

I don't think so, since maybe 'empty' more correctly describes those cases.

I think it depends on how many classes you are thinking to rename. I'm fine by doing it in this PR if it's small enough.

I think that would be more appropriate to do in a different PR if at all.

Thanks for review!

Copy link
Copy Markdown
Contributor

@jihoonson jihoonson left a comment

Choose a reason for hiding this comment

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

Thanks. LGTM!

@jihoonson jihoonson merged commit a99f0ff into apache:master Apr 2, 2019
@jihoonson jihoonson added this to the 0.15.0 milestone Apr 2, 2019
@clintropolis clintropolis deleted the consistent-noop-agg-naming branch April 2, 2019 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consistent naming of 'no-op', 'empty' aggregators

2 participants