-
Notifications
You must be signed in to change notification settings - Fork 698
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
Update visual tree verification infra to get rid of the term "master" #2725
Update visual tree verification infra to get rid of the term "master" #2725
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.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@chingucoding have you tested updating the visual verification reference files after this change? |
I have not tested updating file from the CI. Also we would probably need new screen shots for those since they don't have the new folder name. |
@StephenLPeters Should I revert my changes to the documentation and let #2660 include those changes to prevent merge conflicts? @Felix-Dev Would you be fine with that? |
@chingucoding I'm fine with merging your changes into my PR and fixing any occuring merge conflicts in the developer documentation file (and update it accordingly to use the new term where I added new content). |
Does this have to be done in a subsequent checkin or can we do it atomically? |
Since there are no visual tree verification updates, and the only screenshot right now is the azure artifacts view I don't have access too, there is no screenshot I can update here. If someone with access to the azure directory can provide a screenshot for the updated "microsoft employee" view, we can update it with this PR. However currently the "VisualTree" folder is not present in the drop since there were no changes to the visual tree verification files. |
However since #2660 also introduces new screenshots that also need to be updated, we could also do it with that PR is @Felix-Dev is fine with that. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@chingucoding and @Felix-Dev how should we progress with merging in this and #2660? I think merging this and updating #2660 would make sense? |
@StephenLPeters Yes, we can merge this into master first and then update #2660 accordingly. |
Description
Update documentation, CI scripts and testing code to use new names instead of "master".
Motivation and Context
#2676
How Has This Been Tested?
Ran tests