-
Notifications
You must be signed in to change notification settings - Fork 373
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
samples(storagecontrol): add storagecontrol managed folder samples #14356
Conversation
Here is the summary of changes. You are about to add 4 region tags.
This comment is generated by snippet-bot.
|
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.
The code looks great, thanks.
I am sad that we are spending time on this type of example. We should be thinking about either (1) using snippetgen, or (2) write a product-neutral, and resource-neutral examples. That is, we should have an example that is the prototypical "CreateFoo()", and another for "DeleteFoo()" and another for "Using LROs".
@coryan I agree. These are almost an exact copy of the folder samples I did last week with a few names changed around. I was having some side conversation recently because some languages (i.e. node) already have generated samples for folders / managed folders but we are still hand re-writing them. It seems like we could spend the same time improving the toolset that is generating samples so that they are auto generated to our standard and then bring the tools / samples across all languages. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14356 +/- ##
==========================================
+ Coverage 93.19% 93.68% +0.48%
==========================================
Files 2191 2280 +89
Lines 192367 203329 +10962
==========================================
+ Hits 179282 190480 +11198
+ Misses 13085 12849 -236 ☔ View full report in Codecov by Sentry. |
Fixes #14305
This change is