Skip to content

fix(examples): fix errors in logger and metrics examples #509

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

Merged
merged 3 commits into from
Jan 26, 2022

Conversation

michaelbrewer
Copy link
Contributor

@michaelbrewer michaelbrewer commented Jan 25, 2022

Description of your changes

  • Use shortened imports where possible
  • Use 'serviceName' instead of 'service'
  • Correct some of the relative path imports to work again
  • Correct spacing around '}'
  • Typo in 'metrics/examples/hello-world.ts'
  • Correct link to Tracer examples from Readme

How to verify this change

Run npm test and open and fix examples in the metrics and logger packages.

Related issues, RFCs

PR status

Is this ready for review?: YES
Is it a breaking change?: NO

Checklist

  • My changes meet the tenets criteria
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in areas that should be flagged with a TODO, or hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • The code coverage hasn't decreased
  • I have added tests that prove my change is effective and works
  • New and existing unit tests pass locally and in Github Actions
  • Any dependent changes have been merged and published in downstream module
  • The PR title follows the conventional commit semantics

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Changes:
- Use shortened imports where possible
- Use 'serviceName' instead of 'service'
- Correct some of the relative path imports to work again
- Correct spacing around '}'
- Typo in 'metrics/examples/hello-world.ts'
@@ -17,6 +17,6 @@ const lambdaHandler = async (): Promise<void> => {
};

const handlerWithMiddleware = middy(lambdaHandler)
.use(logMetrics(metrics}));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an actual compile error in TS

@github-actions github-actions bot added the bug Something isn't working label Jan 25, 2022
@dreamorosi dreamorosi added this to the production-ready-release milestone Jan 25, 2022
@dreamorosi dreamorosi requested a review from flochaz January 25, 2022 09:06
@michaelbrewer
Copy link
Contributor Author

@dreamorosi @flochaz - maybe in the long term, some of these examples will be dropped or added to the test harness or move to a separate repo :)

@flochaz
Copy link
Contributor

flochaz commented Jan 25, 2022

Can you open an issue to explain first what is the problem ?

@flochaz
Copy link
Contributor

flochaz commented Jan 25, 2022

And let's try to follow contribution guidelines and have PR fixing one thing issue at a time. thx !

@michaelbrewer
Copy link
Contributor Author

michaelbrewer commented Jan 25, 2022

Ok. These examples have typos and errors in them. I can raise issues for it too.

And the link for the tracer example is wrong again.

@michaelbrewer
Copy link
Contributor Author

Can you open an issue to explain first what is the problem ?

#510

@michaelbrewer
Copy link
Contributor Author

@flochaz - there is no explicit call out for an issue before a PR? Also there is alot of typos and issues in this guide referencing python or CDK:

Example of CDK project references:

Screen Shot 2022-01-25 at 10 23 54 AM

Examples of Python references:

Screen Shot 2022-01-25 at 10 28 13 AM

@flochaz
Copy link
Contributor

flochaz commented Jan 26, 2022

Thanks a lot for your contribution and we will make sure we will be clearer in the contributing guide on our expectation and especially on utility of issue creation and PR scope.

Copy link
Contributor

@saragerion saragerion left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution!

@flochaz flochaz merged commit c19b47c into aws-powertools:main Jan 26, 2022
@michaelbrewer michaelbrewer deleted the fix-examples branch January 26, 2022 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants