-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
chore (docs): Made sure CodeBuild and CodeCommit always have the AWS prefix #1707
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but are you sure you didn't miss the ReadMe for CodeCommit in your PR? It only contains changes for the CodeBuild ReadMe.
@@ -222,7 +222,7 @@ new codebuild.PipelineTestAction(this, 'IntegrationTest', { | |||
stage: buildStage, | |||
project, | |||
// outputArtifactName is optional - if you don't specify it, | |||
// the Action will have an undefined `outputArtifact` property | |||
// the Action has an undefined `outputArtifact` property |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would keep the future tense here (this comment is on a construction property, when the object does not exist yet, so using the present tense feels weird here).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I'll revert the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do NOT have to spell out "AWS CodeBuild" on every usage. I have a source that confirms we use "AWS CodeBuild" on the first usage on the page, and afterwards we use the shorter name "CodeBuild". Especially in the README it becomes cumbersome to read.
In fact, I think our README was already compliant since it had "AWS CodeBuild" in the header (though it might be a matter of debate whether it needs to be in running text or not)
I have a link to an authoritative doc if you need proof of this.
Removed tedious AWS prefix after first reference.
I kept one extra "AWS" after the top-level heading. |
Pull Request Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.