-
Notifications
You must be signed in to change notification settings - Fork 5k
fix storage ci.yml issue. #50160
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
fix storage ci.yml issue. #50160
Conversation
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.
Pull Request Overview
This PR simplifies the CI trigger paths for the Storage SDK by removing redundant explicit includes under sdk/storage/ci.yml
.
- Removed explicit DataMovement subfolder paths under
paths.include
- Retained the generic
sdk/storage/
include to cover all subdirectories
@haiyuazhang: You don't provide any context to understand what issue you're running into and how this is intended to fix it. Please ensure that any PR you open - especially for an area outside of your ownership - has context so that it can be reviewed. //cc: @ArthurMa1978 |
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.
Why do you need this change?
I updated the PR description to highlight the issue this PR intends to address. please help to take a look. |
The Storage DataMovement packages are intentionally included in the Storage CI (to also run for the pullrequest pipeline), due to the fact that changes in Storage tends to affect the DataMovement package. That way the storage team can recognize if a change made in the Storage Base packages has directly affected DataMovement. Including DataMovement in the storage CI has prevented us from having the DataMovement package CI from being broken (which did block other teams from releasing). Is there any way we can still include Storage DataMovement in the Storage Pipeline, without breaking the Update-Mgmt-CI.ps1 script? |
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.
@haiyuazhang: This is the wrong approach. I commented in the issue itself, but if the management script fails because a library makes customizations to their ci configuration - then you need to fix the script.
You cannot assume that the management tooling can dictate the format of configuration for all libraries.
Contributing to the Azure SDK
Please see our CONTRIBUTING.md if you are not familiar with contributing to this repository or have questions.
For specific information about pull request etiquette and best practices, see this section.
fix issue 50137.
As mentioned on the issue page, the current format of the storage ci.yml file does not work well with one script used in the Auto spec PR generation. As For the change itself, since the path sdk/storage is already included, I assume it's safe to delete the other include paths.