-
Notifications
You must be signed in to change notification settings - Fork 34
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
Include aws beam core as a dep #166
Conversation
- This change moves the following files to a common lib: - aws_client - aws_util - aws_request - aws_rds_iam_token - aws_s3_presigned_url In particular the latter two may be very useful for libraries that do not need all of aws_erlang
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 4 out of 10 changed files in this pull request and generated 2 comments.
Files not reviewed (6)
- src/aws.app.src: Language not supported
- src/aws_client.erl: Language not supported
- src/aws_rds_iam_token.erl: Language not supported
- src/aws_request.erl: Language not supported
- src/aws_s3_presigned_url.erl: Language not supported
- src/aws_util.erl: Language not supported
Comments suppressed due to low confidence (2)
README.md:14
- [nitpick] The word 'hence' is used incorrectly. It should be 'therefore' or 'thus.'
Any changes that need to be made should hence be made through [aws_beam_core](https://github.com/aws-beam/aws_beam_core) or [aws-codegen](https://github.com/aws-beam/aws-codegen) :exclamation:
README.md:128
- [nitpick] The word 'Hence' is used incorrectly. It should be 'Therefore' or 'Thus.'
Unfortunately the docs generated are too big for hexdocs.pm. Hence, the docs can be generated locally using:
@@ -1,7 +1,7 @@ | |||
{erl_opts, [nowarn_unused_type, debug_info, {d, maps_support}]}. | |||
{deps, [{hackney, "1.18.0"}, | |||
{jsx, "3.0.0"}, | |||
{aws_signature, "0.3.3"} | |||
{aws_beam_core, "1.0.0"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not important for this PR: Since you are using hex.pm, you can use semver-like syntax
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a 🐔 when it comes to the semver-like syntax. Even though aws_beam_core
is owned by us, I'd prefer hard-pinning it just to ensure that updated dependencies are explicitly bumped rather than implicitly.
Include aws_beam_core as dependency of aws-erlang
AWS Beam Core is a sub-project of the larger AWS Beam projects. The aim of this project is to consolidate common functions into one place, eliminating the need to implement the same functionality in multiple places. It also resulted in
aws-erlang
being a place to ONLY hold generated code without any hand-written code left behind inaws-erlang
.Initially, AWS Beam Core will be integrated into AWS-Erlang, with plans to integrate into AWS-Elixir in the future.
This change moves the following files to the common lib:
- aws_client
- aws_util
- aws_request
- aws_rds_iam_token
- aws_s3_presigned_url
In particular the latter two may be very useful for libraries that do not need all of aws_erlang. Since they're simply moved, backwards compatibility is maintained with only the added dependency as a requirement.