-
Notifications
You must be signed in to change notification settings - Fork 874
add shapes to S3NeedsCustomUpdate and add devconfig #4149
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
add shapes to S3NeedsCustomUpdate and add devconfig #4149
Conversation
stack-info: PR: #4145, branch: peterrsongg/petesong/phase-3-pr-3/1
stack-info: PR: #4146, branch: peterrsongg/petesong/phase-3-pr-3/2
stack-info: PR: #4147, branch: peterrsongg/petesong/phase-3-pr-3/3
stack-info: PR: #4148, branch: peterrsongg/petesong/phase-3-pr-3/4
stack-info: PR: #4149, branch: peterrsongg/petesong/phase-3-pr-3/5
875e96a to
663b399
Compare
| { "AnalyticsAndOperator", 2 }, | ||
| { "IntelligentTieringAndOperator", 2} | ||
| }; | ||
| if (customUpdateShapes.TryGetValue(shape.Name, out int membersCount)) |
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.
Why we are only checking the membersCount? What if the docs changed or the changed something with the validation and made them more open?
I know this isn't part of this PR and I'm not sure if I reviewed the first PR and missed it.
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.
hmm that is a good point, a doc change wouldn't trigger this validation. This is a short-term solution though. Longer term we need a trebuchet validator.
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 am ok which this only until the customization flag is no longer automatically set on the feature for the reason you stated where we don't want to have validation failures. Please add a task to add the validator to the S3 project.
Description
Adds complex shapes to the S3NeedsCustomUpdate. Since these shapes are excluded from generation (or heavily customized). I'm backfilling those shapes that I missed in previous PRs so that the generator will throw an exception if any of these shapes are updated. This is a temporary solution. The longer term solution will work as a validator or an acknowledgment in the build system so that we don't mess up our preview build failure metrics.
Motivation and Context
Testing
Dry run for base branch passed
Screenshots (if appropriate)
Types of changes
Checklist
License