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

cur-setup-destination Readme a bit confusing #839

Open
mthwbarb opened this issue Jun 13, 2024 · 4 comments
Open

cur-setup-destination Readme a bit confusing #839

mthwbarb opened this issue Jun 13, 2024 · 4 comments
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@mthwbarb
Copy link

The cur-setup-destination README example usage shows both the setup-source and setup-destination modules being called. But if you follow the deployment steps the setup-destination would have already been run in step 1 (cur-setup-destination)

https://github.com/aws-samples/aws-cudos-framework-deployment/blob/main/terraform-modules/cur-setup-destination/README.md

@iakov-aws
Copy link
Collaborator

@sean-nixon please can you check this?

@sean-nixon
Copy link
Contributor

@mthwbarb The README you linked for the cur-setup-destination module only contains the cur-setup-destination in the example. Can you clarify where the confusion lies? If there's a way we can make the documentation better, I'd love to hear suggestions.

The README for each module is intended to be more or less standalone documentation for just that one module. The top-level README contains instructions and examples for using all of the modules together to setup CUR and deploy CID dashboards.

@mthwbarb
Copy link
Author

mthwbarb commented Jun 20, 2024

Hi @sean-nixon - I'm referring to the example usage in the CUR SOURCE readme https://github.com/aws-samples/aws-cudos-framework-deployment/blob/main/terraform-modules/cur-setup-source/README.md. In this example, both modules are being provisioned. However, in the deployment steps from the workshop, you would typically deploy CUR DESTINATION first, then CUR SOURCE second, Especially if they are 2 different accounts and you are using 2 different sets of credentials to deploy.

I think the main README is ok but I would remove the CUR DESTINATION module reference from the example in CUR SOURCE readme.

@iakov-aws iakov-aws added the documentation Improvements or additions to documentation label Jun 29, 2024
@sean-nixon
Copy link
Contributor

@mthwbarb Sorry for the delay. I agree with your proposal. I've submitted a PR with some enhancements including removing the CUR destination module from the CUR source module example as well as adding a callout to refer to the main README for complete deployment instructions. Thanks for your feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants