Skip to content

chore(commons): Create a common package #314

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 4 commits into from
Dec 16, 2021
Merged

chore(commons): Create a common package #314

merged 4 commits into from
Dec 16, 2021

Conversation

flochaz
Copy link
Contributor

@flochaz flochaz commented Dec 15, 2021

Description

in order to simplify modules packaging, creating a common module for shared elements such as interfaces, types , test events etc.

Related issues, RFCs

#316

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.

@flochaz flochaz marked this pull request as draft December 15, 2021 10:33
@flochaz flochaz mentioned this pull request Dec 15, 2021
10 tasks
@dreamorosi dreamorosi added the all label Dec 15, 2021
@dreamorosi dreamorosi added this to the beta-release milestone Dec 15, 2021
@dreamorosi
Copy link
Contributor

I know it's a draft, just leaving this here so I don't forget.

Reviewed the files and it's looking good!

Only note is to update the README.md file to match the content of this new common package.

dreamorosi
dreamorosi previously approved these changes Dec 15, 2021
@flochaz flochaz marked this pull request as ready for review December 15, 2021 13:58
}
}
]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can remove all eslint config files outside of the root folder.
I did some research and eslint is smart enough to navigate to all the parent folders to look for a valid config file and use that one. We can leave the one at the root and set the value root = true in the file:
https://eslint.org/docs/user-guide/configuring/configuration-files#cascading-and-hierarchy
After that you can simply do the same in your IDE (webstorm works)

aws-lambda-powertools-typescript
├── package.json
├── .eslintrc -> setting `root` = `true`
├── packages

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep but there is a lot to fix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so préfères to keep it for a dedicated pr

logGroupName: '/aws/lambda/foo-bar-function-123456abcdef',
logStreamName: '2021/03/09/[$LATEST]abcdef123456abcdef123456abcdef123456',
invokedFunctionArn: 'arn:aws:lambda:eu-central-1:123456789012:function:Example',
awsRequestId: 'c6af9ac6-7b61-11e6-9a41-93e8deadbeef',
Copy link
Contributor

@saragerion saragerion Dec 16, 2021

Choose a reason for hiding this comment

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

I made some changes in the middleware branch: removed the deadbeef references and improved consistency of values:
arn:aws:lambda:eu-central-1:123456789012:function:Example -> arn:aws:lambda:eu-central-1:123456789012:function:foo-bar-function

#313

Happy to repeat it in yours if you want

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 Florian! Can you add an example on how can we leveage the shared resources like the tsconfig file in the commons package in another package?

@flochaz
Copy link
Contributor Author

flochaz commented Dec 16, 2021

Thanks Florian! Can you add an example on how can we leveage the shared resources like the tsconfig file in the commons package in another package?

you have a usage example in #316

@flochaz flochaz merged commit 6b5b147 into main Dec 16, 2021
@flochaz flochaz deleted the chore/addCommons branch December 16, 2021 13:45
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.

4 participants