-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
[Storage][Webjobs] Hide blob name validation attribute. #15794
[Storage][Webjobs] Hide blob name validation attribute. #15794
Conversation
@@ -20,7 +20,7 @@ namespace Microsoft.Azure.WebJobs.Description | |||
/// Validate this on the client side so that we can get a user-friendly error rather than a 400 from the service. | |||
/// See code here: http://social.msdn.microsoft.com/Forums/en-GB/windowsazuredata/thread/d364761b-6d9d-4c15-8353-46c6719a3392 | |||
/// </summary> | |||
public class BlobNameValidationAttribute : ValidationAttribute | |||
internal sealed class BlobNameValidationAttribute : ValidationAttribute |
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.
Right - because where these are validated https://github.com/Azure/azure-webjobs-sdk/blob/ccc20de1fb7fd7043aed6d12a2c46acce1d7a49a/src/Microsoft.Azure.WebJobs.Host/Bindings/AttributeCloner.cs#L199 we look for ValidationAttribute.
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.
Did you not carry the PublicSurface tests forward to this new repo? https://github.com/Azure/azure-webjobs-sdk/blob/5e410d7ae1c7c1b54b81b6a50faa5cc226deba5e/test/Microsoft.Azure.Webjobs.Extensions.Storage.UnitTests/PublicSurfaceTests.cs#L55. We had a set of useful tests that helped us verify public surface area.
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 did initially (See #15381 (comment) ). However the azure-sdk-for-net
has an infrastructure to extract public surface into files like /api/Azure.WebJobs.Extensions.Storage.Blobs.netstandard2.0.cs
(included in this diff) and warn against public area changes.
There's also a process to review public surface area before shipping any public preview or GA which is based on these extracts.
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.
Here's more about the process https://github.com/Azure/azure-sdk/blob/master/docs/policies/reviewprocess.md
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.
👍
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.
Sounds good, thanks
In this PR: