Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Make pipeline protected #3934

Merged
merged 2 commits into from
Jun 19, 2019
Merged

Make pipeline protected #3934

merged 2 commits into from
Jun 19, 2019

Conversation

jeremymeng
Copy link
Member

No description provided.

@jeremymeng jeremymeng requested a review from XiaoningLiu June 18, 2019 23:36
@jeremymeng
Copy link
Member Author

One feedback from the ArchBoard is to make pipeline protected. I don't know whether there is still strong reason to do so after hiding StorageClient. @XiaoningLiu what do you think?

@jeremymeng jeremymeng self-assigned this Jun 18, 2019
@jeremymeng jeremymeng changed the base branch from feature/storage to master June 19, 2019 02:49
@jeremymeng jeremymeng changed the base branch from master to feature/storage June 19, 2019 02:49
XiaoningLiu
XiaoningLiu approved these changes Jun 19, 2019
View reviewed changes
Copy link
Member

@XiaoningLiu XiaoningLiu left a comment

Choose a reason for hiding this comment

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

Generally sounds good to me. Though it lead to test case changes. The reason I think is to make sure pipeline is immutable. For advanced customers who creates a Client with customized pipeline, they can keep the pipeline instance.

@jeremymeng
Copy link
Member Author

Hmm the LeaseClient is using pipeline...

@jeremymeng
Fix build error
4900336 
The only place in lib code where pipeline is used.
@jeremymeng jeremymeng merged commit 0a8408f into Azure:feature/storage Jun 19, 2019
@jeremymeng jeremymeng deleted the protect-pipeline branch June 19, 2019 16:29
This was referenced Nov 23, 2021
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XiaoningLiu XiaoningLiu XiaoningLiu approved these changes

Assignees

@jeremymeng jeremymeng

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jeremymeng @XiaoningLiu