search

blimmer / cdk-datadog-integration

Amazon Cloud Development Kit (CDK) logic to integrate your AWS account with Datadog
Apache License 2.0
19 stars 8 forks source link

fix: Change addDependsOn to addDependency #52

Closed hlascelles closed 1 year ago

hlascelles commented 1 year ago

addDependsOn is deprecated:

❯ yarn cdk synth
info: Preparing App...
[WARNING] aws-cdk-lib.CfnResource#addDependsOn is deprecated.
  use addDependency
  This API will be removed in the next major release.
[WARNING] aws-cdk-lib.CfnResource#addDependsOn is deprecated.
  use addDependency
  This API will be removed in the next major release.
blimmer commented 1 year ago

Hey @hlascelles - thanks for this PR. As you can see from the build failure, this change will also require an update to the minimum version of CDK v2. Currently, that's 2.1.0: https://github.com/blimmer/cdk-datadog-integration/blob/e255fa07a00cb98d03c48a141e6b5c44e55dee70/.projenrc.js#L6

2.1.0 is quite old, so I think it's safe to bump to the minimum version of CDK that supports addDependency. You'll need to look in the changelogs to see when that was released and make the appropriate update.

Thanks for your contribution!

hlascelles commented 1 year ago

:+1: Looks like v2.56.0 https://github.com/aws/aws-cdk/commit/ecedb00ee3a3cfcaa2564a679fa635aff38f32d8

blimmer commented 1 year ago

Since this uses projen, you'll need to update the version in projenrc.js (linked in my comment above) and then run yarn projen. Commit the result and we can run the test suite. Thanks!

hlascelles commented 1 year ago

I struggled to do a yarn install as I get errors:

➤ YN0066: │ typescript@patch:typescript@npm%3A4.8.3#builtin<compat/typescript>::version=4.8.3&hash=8133ad: Cannot apply hunk #2 (set enableInlineHunks for details)
➤ YN0000: └ Completed in 11s 770ms
➤ YN0000: Failed with errors in 22s 702ms

Then if I try yarn set version latest I think I get further, but then yarn projen (with the new library version) gives Unknown Syntax Error: Unsupported option name ("--check-files").

I must admit this is all beyond my ken. I can't proceed this PR further, sorry!

blimmer commented 1 year ago

Fixed by #54 - I explained the changes over there. Thanks for your contribution! I'll close this PR in favor of the other one.