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.

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

Merged
merged 5 commits into from
Jun 30, 2020

Conversation

marcelwgn
Copy link
Collaborator

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

@msft-github-bot msft-github-bot added the needs-triage Issue needs to be triaged by the area owners label Jun 21, 2020
Copy link
Contributor

@StephenLPeters StephenLPeters left a comment

Choose a reason for hiding this comment

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

:shipit:

@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@StephenLPeters
Copy link
Contributor

@chingucoding have you tested updating the visual verification reference files after this change?

@marcelwgn
Copy link
Collaborator Author

@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 StephenLPeters added area-DevInternal Internal build infrastructure, code cleanup, engineering efficiency team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Jun 22, 2020
@marcelwgn
Copy link
Collaborator Author

marcelwgn commented Jun 22, 2020

@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?

@Felix-Dev
Copy link
Contributor

Felix-Dev commented Jun 22, 2020

@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).

@StephenLPeters
Copy link
Contributor

@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.

Does this have to be done in a subsequent checkin or can we do it atomically?

@marcelwgn
Copy link
Collaborator Author

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.
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.

@marcelwgn
Copy link
Collaborator Author

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.

@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@StephenLPeters
Copy link
Contributor

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.

@chingucoding and @Felix-Dev how should we progress with merging in this and #2660? I think merging this and updating #2660 would make sense?

@Felix-Dev
Copy link
Contributor

@StephenLPeters Yes, we can merge this into master first and then update #2660 accordingly.

@StephenLPeters StephenLPeters merged commit 3834f4d into microsoft:master Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-DevInternal Internal build infrastructure, code cleanup, engineering efficiency team-Controls Issue for the Controls team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants