-
Notifications
You must be signed in to change notification settings - Fork 30
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
chore: fix some code style #280
Conversation
@@ -34,7 +34,6 @@ export const DefaultDatadogProps = { | |||
enableMergeXrayTraces: false, | |||
injectLogContext: true, | |||
enableDatadogLogs: true, | |||
architecture: "X86_64", |
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.
Removing unused const
log.debug("Granting read access to the provided Secret ARN for all your lambda functions."); | ||
grantReadLambdasFromSecretArn(construct, this.props.apiKeySecretArn, extractedLambdaFunctions); | ||
} | ||
if (extractedLambdaFunctions.length === 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.
this.props
can't beundefined
, so removing this check.- flip the if condition and move the large chunk of code outside the if block
const isARM = | ||
lam.architecture?.dockerPlatform !== undefined && | ||
lam.architecture.dockerPlatform === Architecture.ARM_64.dockerPlatform; | ||
const isARM = lam.architecture?.dockerPlatform === Architecture.ARM_64.dockerPlatform; |
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.
Consolidating the two conditions
lambdaLayerArn = getLambdaLayerArn(region, dotnetLayerVersion, runtime, isARM, accountId); | ||
log.debug(`Using dd-trace-dotnet layer: ${lambdaLayerArn}`); | ||
addLayer(lambdaLayerArn, false, scope, lam, runtime); | ||
switch (lambdaRuntimeType) { |
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.
Changing to switch-case
@@ -86,6 +86,7 @@ eslintConfig.addOverride("extends", [ | |||
]); | |||
|
|||
eslintConfig.addOverride("rules", { | |||
"@typescript-eslint/switch-exhaustiveness-check": "error", |
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.
Lint rule for exhaustive switch-case
/merge |
🚂 MergeQueue: pull request added to the queue The median merge time in Use |
What does this PR do?
Make some fixes around code style
RuntimeType
some day, the switch-case will become non-exhaustive and lint will remind us to handle the new value in the switch-caseto
to make code more readable.
See comments on code for more.
Motivation
Testing Guidelines
npx projen build
finishes successfully.To test the link exhaustive check, I removed
from the switch-case block, then ran
![Pasted Graphic 4](https://private-user-images.githubusercontent.com/10097700/358396490-5804b0b6-48c3-4152-8bd5-b7ff77d8428e.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk2NTQwODEsIm5iZiI6MTczOTY1Mzc4MSwicGF0aCI6Ii8xMDA5NzcwMC8zNTgzOTY0OTAtNTgwNGIwYjYtNDhjMy00MTUyLThiZDUtYjdmZjc3ZDg0MjhlLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE1VDIxMDk0MVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTE2N2ExYjBkM2JlMzAxY2FkOGU1MGFkZTgxY2EyNGU4MGYwMDVhYWJjMzUxMjg4ODA3M2QxMTFlNWNhYTI3MjQmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.uWqAPe9e-WwaH0_AYfVCgjsnlXddg4xr2bValiWwd_0)
npx projen build
. I got an error as expected:Additional Notes
Types of Changes
Check all that apply