Skip to content

Conversation

@martinschaef
Copy link
Contributor

Description of changes:

Update to README and remove requirement that bucket names start with codeguru-reviewer-*

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

README.md Outdated
### Using Encryption

CodeGuru Reviewer allows you to use a customer managed key (CMCMK) to encrypt content of the S3 bucket that is used
store source and build artifacts, and all metadata and recommendations that are produced by CodeGuru Reviewer.

Choose a reason for hiding this comment

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

"to store"?

throw new GuruCliException(ErrorCodes.BAD_BUCKET_NAME,
config.getBucketName() + " is not a valid bucket name for CodeGuru.");
Log.warn("CodeGuru Reviewer has default settings only for buckets that are prefixed with "
+ "codeguru-reviewer. If you choose a different, read the instructions in the README.");

Choose a reason for hiding this comment

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

different what?

Copy link

@senguptaaritra senguptaaritra left a comment

Choose a reason for hiding this comment

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

Lgtm

@martinschaef martinschaef merged commit ee187b2 into aws:main Feb 3, 2022
push:
branches:
- main
on: [push, pull_request, workflow_dispatch]
Copy link
Contributor

Choose a reason for hiding this comment

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

Last time I had both of push and pull_request enabled (unrestricted, like here, for both events in all branches), I quickly realized that doing so triggered a LOT of analyses. Every push to a feature branch with an open PR also counts as a pull_request event, so you end up running 2x Guru for each push to any PR.

Have you considered unrestricted pull_request, but push only for main?

accepts bucket names that start with the prefix `codeguru-reviewer-`.
supports bucket names that start with the prefix `codeguru-reviewer-` out of the box. If you choose a different naming
pattern for your bucket you need to:
1. Grant `S3:GetObject` permissions on their S3 bucket to `codeguru-reviewer.amazonaws.com`
Copy link
Contributor

Choose a reason for hiding this comment

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

s/their/the/ ?

supports bucket names that start with the prefix `codeguru-reviewer-` out of the box. If you choose a different naming
pattern for your bucket you need to:
1. Grant `S3:GetObject` permissions on their S3 bucket to `codeguru-reviewer.amazonaws.com`
2. If you are using SSE on the S3 bucket, Grant `KMS::Decrypt` permissions to `codeguru-reviewer.amazonaws.com`
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: s/Grant/grant/


### Using Encryption

CodeGuru Reviewer allows you to use a customer managed key (CMCMK) to encrypt content of the S3 bucket that is used
Copy link
Contributor

Choose a reason for hiding this comment

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

s/to encrypt content of the/to encrypt the content of the/

README.md Outdated
### Using Encryption

CodeGuru Reviewer allows you to use a customer managed key (CMCMK) to encrypt content of the S3 bucket that is used
store source and build artifacts, and all metadata and recommendations that are produced by CodeGuru Reviewer.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/used store/used to store/


CodeGuru Reviewer allows you to use a customer managed key (CMCMK) to encrypt content of the S3 bucket that is used
store source and build artifacts, and all metadata and recommendations that are produced by CodeGuru Reviewer.
First, create a customer owned key in KMS.
Copy link
Contributor

Choose a reason for hiding this comment

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


```json
{
"Sid": "Allow CodeGuru to use the key to decrypt artifact",
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: replace "artifact" with "artifacts" (or with "the artifact" if just one)

}
}
```
Then, enable server-side for the bucket that you are using with CodeGuru Reviewer. The bucket name should be
Copy link
Contributor

Choose a reason for hiding this comment

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

s/enable server-side/enable server-side encryption/

throw new GuruCliException(ErrorCodes.BAD_BUCKET_NAME,
config.getBucketName() + " is not a valid bucket name for CodeGuru.");
Log.warn("CodeGuru Reviewer has default settings only for buckets that are prefixed with "
+ "codeguru-reviewer. If you choose a different, read the instructions in the README.");
Copy link
Contributor

Choose a reason for hiding this comment

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

s/different/different name/

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants