Skip to content

chore(toolkit): cleanup public interfaces#105

Merged
aws-cdk-automation merged 2 commits intomainfrom
mrgrain/chore/toolkit-cleanup
Feb 24, 2025
Merged

chore(toolkit): cleanup public interfaces#105
aws-cdk-automation merged 2 commits intomainfrom
mrgrain/chore/toolkit-cleanup

Conversation

@mrgrain
Copy link
Contributor

@mrgrain mrgrain commented Feb 24, 2025

Clean-up public interface and docs generation in preparation of the release.

This mostly moves some previously public interfaces to be private, renames interface and removes unused options.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license


toolkitLib.addTask('docs', {
exec: 'typedoc lib/index.ts --excludeExternals --excludePrivate --excludeProtected --excludeInternal',
exec: 'typedoc lib/index.ts',
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 now all in typedoc.json

@codecov-commenter
Copy link

codecov-commenter commented Feb 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.36%. Comparing base (f25e658) to head (8c119fb).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #105      +/-   ##
==========================================
- Coverage   84.44%   84.36%   -0.09%     
==========================================
  Files         196      196              
  Lines       35089    35089              
  Branches     4509     4506       -3     
==========================================
- Hits        29632    29603      -29     
- Misses       5320     5342      +22     
- Partials      137      144       +7     
Flag Coverage Δ
suite.unit 84.36% <100.00%> (-0.09%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Enum values need to be the same in CLI and Toolkit Lib. This option is not publicly exposed as a CLI option, so changing the value is okay

Copy link
Contributor

@rix0rrr rix0rrr Feb 24, 2025

Choose a reason for hiding this comment

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

I would advise against load-bearing enum values to begin with. Just because TypeScript allows you to do it doesn't mean it's a good idea. If you need to translate between two enums that are structurally but not nominally the same, the boring and annoying (but safe and correct) way to do it would be:

function enumOneToTwo(x: my.AssetBuildTime): their.AssetBuildTime {
  switch (x) { 
    case my.AssetBuildTime.VALUE_ONE: return their.AssetBuildTime.VALUE_ONE;
    case my.AssetBuildTime.VALUE_TWO: return their.AssetBuildTime.VALUE_TWO;
    // ...
  }
}

@rix0rrr rix0rrr disabled auto-merge February 24, 2025 11:30
@mrgrain mrgrain enabled auto-merge February 24, 2025 11:41
@rix0rrr rix0rrr added the pr/exempt-integ-test Skips the integ test steps if set. label Feb 24, 2025
@rix0rrr rix0rrr marked this pull request as draft February 24, 2025 11:58
auto-merge was automatically disabled February 24, 2025 11:58

Pull request was converted to draft

@rix0rrr rix0rrr marked this pull request as ready for review February 24, 2025 11:58
@aws-cdk-automation aws-cdk-automation added this pull request to the merge queue Feb 24, 2025
Merged via the queue into main with commit 6b8cceb Feb 24, 2025
12 checks passed
@aws-cdk-automation aws-cdk-automation deleted the mrgrain/chore/toolkit-cleanup branch February 24, 2025 12:46
github-merge-queue bot pushed a commit that referenced this pull request Feb 24, 2025
In #105 I renamed the interface from `Extend` to `Expand`, but forgot
the options itself and some docs.

This is marked as a chore so it doesn't show up in the changelog for the
first release. That's what we want, because it is not relevant to users
just yet.

---
By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr/exempt-integ-test Skips the integ test steps if set.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants