Skip to content

[Core] Documentation for pipeline and policies#20418

Merged
joheredi merged 18 commits intoAzure:mainfrom
joheredi:pipeline-docs
Feb 25, 2022
Merged

[Core] Documentation for pipeline and policies#20418
joheredi merged 18 commits intoAzure:mainfrom
joheredi:pipeline-docs

Conversation

@joheredi
Copy link
Copy Markdown
Member

Packages impacted by this PR

None. This PR adds documentation for Pipeline and PipelinePolicy

Issues associated with this PR

#13561

Describe the problem that is addressed by this PR

Add documentation for the Pipeline and Policies and how to customize them.

@ghost ghost added the Azure.Core label Feb 16, 2022
@joheredi joheredi changed the title [Core] Document pipeline and policies [Core] Documentation for pipeline and policies Feb 16, 2022
Copy link
Copy Markdown
Member

@xirzec xirzec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for writing this up! 👍

This is a fantastic starting point and I am glad we will soon have this checked into the repo for folks to reference.

I left some pretty high-level feedback, since I figured we can debate periods and grammar once the content has settled more. 😄

Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md
Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md Outdated
Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md Outdated
Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md Outdated
Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md
Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md
Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md
Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md
Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md Outdated
Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md
@joheredi joheredi requested a review from ramya-rao-a February 17, 2022 21:27
Copy link
Copy Markdown
Contributor

@ramya-rao-a ramya-rao-a left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General thought: Should this be combined with the readme for core-rest-pipeline instead of a separate document?

Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md Outdated
Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md Outdated
Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md Outdated
Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md Outdated
Copy link
Copy Markdown
Member

@xirzec xirzec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! 👍

Left some small grammar nits / typo suggestions as well as a couple expanded details where I thought it would be helpful.

Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md Outdated
Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md Outdated
Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md Outdated
Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md Outdated
Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md Outdated
Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md Outdated
Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md Outdated
Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md Outdated
Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md Outdated
Comment thread sdk/core/core-rest-pipeline/documentation/pipeline.md Outdated
@xirzec
Copy link
Copy Markdown
Member

xirzec commented Feb 23, 2022

General thought: Should this be combined with the readme for core-rest-pipeline instead of a separate document?

This is a good idea, or if we think it is too meaty of a topic, we should certainly modify the core-rest-pipeline readme to emphatically link to here.

Copy link
Copy Markdown
Member

@jeremymeng jeremymeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding the docs! I only have nit comments.

@@ -0,0 +1,251 @@
# Policies
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it is better to start with some general sentence about us using a list of request policy (pipeline) in our sdk, then followed by sections in details?


# Pipelines

A `Pipeline` composes several policies and provides the ability to make a request that will flow through the policies in a specific order determined by their requirements. Which allows manipulating each request before it is sent to the server and to work on the response after receiving it.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"composes" => "is composed of"?

joheredi and others added 15 commits February 24, 2022 10:45
Co-authored-by: Ramya Rao <ramya.rao.a@outlook.com>
Co-authored-by: Ramya Rao <ramya.rao.a@outlook.com>
Co-authored-by: Ramya Rao <ramya.rao.a@outlook.com>
Co-authored-by: Jeff Fisher <xirzec@xirzec.com>
Co-authored-by: Jeff Fisher <xirzec@xirzec.com>
Co-authored-by: Jeff Fisher <xirzec@xirzec.com>
Co-authored-by: Jeff Fisher <xirzec@xirzec.com>
Co-authored-by: Jeff Fisher <xirzec@xirzec.com>
Co-authored-by: Jeff Fisher <xirzec@xirzec.com>
Co-authored-by: Jeff Fisher <xirzec@xirzec.com>
Co-authored-by: Jeff Fisher <xirzec@xirzec.com>
Co-authored-by: Jeff Fisher <xirzec@xirzec.com>
@joheredi joheredi merged commit 4882b4a into Azure:main Feb 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants