Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename Enumerable to Stream #26

Merged
merged 13 commits into from
Aug 14, 2019
Merged

Rename Enumerable to Stream #26

merged 13 commits into from
Aug 14, 2019

Conversation

sam-goodwin
Copy link
Owner

@sam-goodwin sam-goodwin commented Aug 13, 2019

Fixes #25

  • Rename Enumerable to Stream
  • Move services into their own namespace:
    • Glue
    • DynamoDB
    • SNS
    • SQS
    • Kinesis
    • S3
    • Lambda
  • Update Examples
  • Update README

@@ -120,7 +120,7 @@ const stream = queue.enumerable() // enumerable gives us a nice chainable API fo
};
}
})
.toStream(stack, 'Stream', {
.toKinesis(stack, 'Stream', {
Copy link
Owner Author

Choose a reason for hiding this comment

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

Any thoughts on toKinesis vs toKinesisStream? I.e. should I include the resource type as a suffix?

  • toSQS or toSQSQueue
  • toSNS or toSNSTopic
  • toKinesis or toKinesisStream
  • toS3DeliveryStream or toFirehose
  • toGlue or toGlueTable (note: it's already toGlueTable, so we need to change it for consistency reasons)

Copy link
Contributor

Choose a reason for hiding this comment

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

it makes sense just to be a little more verbose to where it's going to, LGTM.

to{Service}{Resource} is a good convention, makes it easier to reason. thinking about those services such as SageMaker that have a lot of underlying resources (although couldn't visualize a toSageMaker{Resource} from the top of my head)

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yeah, I think this is a good convention. Updated to reflect that.

I'm not that happy with toS3DeliveryStream. Could have gone with toFirehoseDeliveryStream, but how to differentiate between destinations (Redshift, ElasticSearch, Splunk, etc.) Created #31 to follow up.

@sam-goodwin sam-goodwin changed the title [WIP] rename Enumerable to Stream Rename Enumerable to Stream Aug 13, 2019
@sam-goodwin sam-goodwin added the stream related to the stream chainable API label Aug 13, 2019
@sam-goodwin
Copy link
Owner Author

While we're namespacing all the things, should Function => Lambda.Function?

import * as DynamoDB from './dynamodb';

export {
DynamoDB
Copy link
Owner Author

Choose a reason for hiding this comment

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

This was a neat trick!

@pocesar
Copy link
Contributor

pocesar commented Aug 13, 2019

While we're namespacing all the things, should Function => Lambda.Function?

yeah, Function is a global constructor for function, would be good to go ahead and namespace that as well, and make it obvious it's a Lambda Function (and even sounds cooler haha)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stream related to the stream chainable API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

enumerable is a bad name
2 participants