Skip to content
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

update GuRole import in instance role #208

Merged
merged 1 commit into from
Jan 27, 2021
Merged

Conversation

jamie-lynch
Copy link
Contributor

What does this change?

This PR updates the import of GuRole in the instance-role.ts file to import directly from the roles.ts file rather than the index.ts in the roles directory as this was causing an error when importing anything from @guardian/cdk/lib/constructs/iam.

Does this change require changes to existing projects or CDK CLI?

Any projects importing GuInstanceRole or GuRole from their specific file will now be able to get them from the @guardian/cdk/lib/constructs/iam path.

How to test

Bump to the latest version and try importing the GuInstanceRole construct from @guardian/cdk/lib/constructs/iam

How can we measure success?

All imports work as expected.

@jamie-lynch jamie-lynch added the needs-new-release Identifying significant changes to the library label Jan 26, 2021
@jamie-lynch jamie-lynch requested a review from a team January 26, 2021 16:30
@jamie-lynch
Copy link
Contributor Author

It looks like this issue didn't show up in our tests as the classes were only ever imported directly from their files. One thing that might be useful is to include some example stacks in this repository and try to build them during CI as an integration test.

As a side-effect, keeping these examples up to date alongside feature work might make it easier for users to see what a current "best practice" stack looks like and what changes are needed to incorporate new features. It would also give us a good test bed to trial features on as we work on them, rather than having to deploy and bump the version used in external projects.

I've started #209 to discuss this further.

@jamie-lynch jamie-lynch merged commit 6dbd0f7 into main Jan 27, 2021
@jamie-lynch jamie-lynch deleted the jl/fix-instance-role branch January 27, 2021 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-new-release Identifying significant changes to the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants