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

feat: add cloudfront-s3-cdk-go pattern #2251

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Gryczka
Copy link
Contributor

@Gryczka Gryczka commented Apr 14, 2024

Issue #, if available: #2250

Description of changes: Adding a new pattern for CloudFront to S3 Static Site implemented using the CDK in Go.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link
Contributor

@pputhran pputhran left a comment

Choose a reason for hiding this comment

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

Please refer to the requested changes

cloudfront-s3-cdk-go/example-pattern.json Outdated Show resolved Hide resolved
"gitHub": {
"template": {
"repoURL": "https://github.com/aws-samples/serverless-patterns/tree/main/cloudformation-s3-cdk-go",
"templateURL": "serverless-patterns/cloudformation-s3-cdk-go",
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove the projectFolder value from the templateFile. Please refer to some of the other Go examples that we have under Serverless patterns.

        "template": {
            "repoURL": "https://github.com/aws-samples/serverless-patterns/tree/main/cloudformation-s3-cdk-go",
            "templateURL": "serverless-patterns/cloudformation-s3-cdk-go",
            "projectFolder": "cloudfront-s3-cdk-go",
            "templateFile": "cloudformation-s3-cdk-go.go"
        }
        
        ```

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @pputhran I have removed the projectFolder value and fixed a few cloudformation/cloudfront typos.

But could you point me toward one of the relevant go examples? None of the go cdk examples I see have an example-pattern.json present.

update linkedin profile value

update github template values.
fix typo
removing projectFolder value
@Gryczka
Copy link
Contributor Author

Gryczka commented May 10, 2024

Hi @pputhran I have removed the projectFolder value and fixed a few cloudformation/cloudfront typos.

But could you point me toward one of the relevant go examples? None of the go cdk examples I see have an example-pattern.json present.

@Gryczka
Copy link
Contributor Author

Gryczka commented May 15, 2024

Hi @pputhran just pinging back on whether you have a cdk go example json you could share.

@Gryczka Gryczka requested a review from pputhran May 17, 2024 03:50
]
},
"gitHub": {
"template": {
Copy link
Contributor

Choose a reason for hiding this comment

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

The projectFolder is the required field. Also since the cloudfront-s3-cdk-go.go was renamed, its throwing a new error. Can you rename this file to cloudfronts3-cdk-go.go?

Can you please update as below

        "template": {
            "repoURL": "https://github.com/aws-samples/serverless-patterns/tree/main/cloudformation-s3-cdk-go",
            "templateURL": "serverless-patterns/cloudformation-s3-cdk-go",
            "projectFolder": "cloudfront-s3-cdk-go",
            "templateFile": "cloudfronts3-cdk-go.go"
        }

@pputhran
Copy link
Contributor

Hi @pputhran I have removed the projectFolder value and fixed a few cloudformation/cloudfront typos.

But could you point me toward one of the relevant go examples? None of the go cdk examples I see have an example-pattern.json present.

You are right. I assumed there were a few examples already. My bad. Apologies. I have added my comments and if you can update them, I can then approve it.

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

Successfully merging this pull request may close these issues.

None yet

3 participants