-
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
Add developer driven evolution document #27559
Conversation
This pull request is protected by Check Enforcer. |
Would you please link the PR to the issue by putting "Fixes " in the issue description? |
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 think we can restructure this to simplify it a bit. Instead of enumerating GET/PUT/POST/etc., what if we gave the rules for writing a convenience API?
- Replace the RequestContext parameter with CancellationToken
- Replace the input parameter RequestContent with a model type
- Replace the return value Response with Response<T>
- If you end up with an ambiguous call-site, rename the method
- Add suffix Value or Values
- If this isn't a good name, pick a good name for the method
- Create the models for the convenience APIs you've added
Yes, I do follow this by giving rules, and I added more rules as you listed above. Enumerating GET/PUT/POST/etc. is just examples for how to improve to grow up method. |
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.
Thanks!
We'll want to make sure we sort out the diagnostic scope questions and provide guidance for that before we GA -- @pshao25, would you create a separate issue for this, so we don't block this PR on that?
|
Yes, created here: Azure/autorest.csharp#2115 |
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.
LGTM
/check-enforcer override |
Addresses Azure/autorest.csharp#1945