feat: expose bucketArn/bucketName and domain name getters on SecureBucket (fixes #41)#56
feat: expose bucketArn/bucketName and domain name getters on SecureBucket (fixes #41)#56hanu-14 wants to merge 2 commits into
Conversation
alphacrack
left a comment
There was a problem hiding this comment.
Nice work — plain get accessors are exactly the jsii-friendly shape (they translate to properties in Python/Java/.NET), the doc comments are consistent with the file's style, and including the two domain-name getters covers the "consider" part of #41.
One non-blocking nit: the second test (resolves getters to the same values as the wrapped bucket) asserts a subset of what the first already covers. Consider dropping it, or making it assert something distinct — e.g. that Token.isUnresolved(bucket.bucketArn) is true, which would prove the getter returns the live CDK token. Fine to address here or not at all.
|
Re-checked against the new main: MERGEABLE/CLEAN. Two things verified specifically for today's toolchain moves:
Merges after #54. Ready. |
|
Thanks for the thorough review! I appreciate the verification of jsii 6 compatibility and ESLint 10 cleanliness. The diff looks good as-is, and I’ll merge after #54. |
Adds pass-through readonly getters for bucketArn, bucketName, bucketDomainName, and bucketRegionalDomainName on SecureBucket so common attribute access no longer requires .bucket. Includes tests; jsii build and full suite pass.