chore(glue-alpha): fix typos, inconsistencies, docs #33047
Conversation
| networking constraints for data source connections, and there is ambiguity | ||
| around how to securely store secrets for JDBC connections. Finally, | ||
| developers want prescriptive guidance via best practice defaults for | ||
| throughput parameters like number of workers and batching. |
There was a problem hiding this comment.
this simply is not consistent with the voice of READMEs in the rest of the modules. we do not reference anything beyond how the current L2 constructs can be used. the rest of the README needs work, but this particular paragraph isn't necessary
| * —enable-metrics, —enable-spark-ui, —enable-continuous-cloudwatch-log. | ||
| * You can find more details about version, worker type and other features | ||
| * in Glue's public documentation. | ||
| */ |
There was a problem hiding this comment.
- these are docstrings, which are meant to be attached to a particular code construct.
- there's no reason to document source code like this. much of this can/should be in the README, or a docstring on the particular construct we vend to customers. that way they can see our documentation in their IDE, or in our published docs.
| @@ -46,7 +31,7 @@ export interface PySparkFlexEtlJobProps extends JobProperties { | |||
| * | |||
| * @default - no extra python files specified. | |||
| * | |||
| * @see `--extra-py-files` in https://docs.aws.amazon.com/glue/latest/dg/aws-glue-programming-etl-glue-arguments.html | |||
There was a problem hiding this comment.
@see is going to think the entire line is a link, so we can't have additional qualifiers unfortunately.
| * | ||
| * @param scope | ||
| * @param id | ||
| * @param props |
There was a problem hiding this comment.
i have no problem with these doctags, if they were filled out. i'm also happy to omit them entirely, so just deleting here
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #33047 +/- ##
=======================================
Coverage 81.49% 81.49%
=======================================
Files 224 224
Lines 13755 13755
Branches 2413 2413
=======================================
Hits 11209 11209
Misses 2273 2273
Partials 273 273
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Comments on closed issues and PRs are hard for our team to see. |
fixing some fast-follow items from the recent glue PR that was merged. there is more work to be done specifically around the README but I see this as the minimum amount of changes to make glue-alpha somewhat consistent with the rest of the modules we offer in cdk, alpha or not
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license