Skip to content

Update doc for visual tree masters update#2660

Merged
ranjeshj merged 17 commits intomicrosoft:masterfrom
Felix-Dev:user/Felix-Dev/devdoc-update
Jul 13, 2020
Merged

Update doc for visual tree masters update#2660
ranjeshj merged 17 commits intomicrosoft:masterfrom
Felix-Dev:user/Felix-Dev/devdoc-update

Conversation

@Felix-Dev
Copy link
Contributor

@Felix-Dev Felix-Dev commented Jun 14, 2020

Description

The given developer documentation for updating the visual tree masters is not showing the correct steps in case the WinUI contributor does not have permissions to view the Azure Pipelines artifact content online. This PR updates the images accordingly.

Note: I have kept the original images describing how to specifically download the new visual tree masters in Azure Pipelines directly (but removed them from the documentation for now) as I'm not entirely sure who exactly has these online download permissions (just the WinUI team or even more folks)? If required, I will put them into the developer guide again. If they are no longer needed, I will gladly remove them.

How Has This Been Tested?

Visually (see screenshots).

Screenshots:

Old:

image

New:

image

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

@marcelwgn marcelwgn left a comment

Choose a reason for hiding this comment

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

Maybe we could have both situations covered in this doc instead of just showing the "external contributor" workflow, since the two workflows seem different enough to point that out.

@Felix-Dev
Copy link
Contributor Author

Felix-Dev commented Jun 14, 2020

Yes, this is my reasoning why I did not yet delete the old images. Since I don't know who exactly has the permission to online download the visual tree masters I will defer to the WinUI team here. (For example, if only the WinUI team has permission to download them online, a case can be made that documenting this interally then would be enough since the info would be of little to no use for the community contributors. If, however, folks external to the WinUI team can also have these permissions then this case should be covered in the documentation.)

@StephenLPeters
Copy link
Contributor

Yes, this is my reasoning why I did not yet delete the old images. Since I don't know who exactly has the permission to online download the visual tree masters I will defer to the WinUI team here. (For example, if only the WinUI team has permission to download them online, a case can be made that documenting this interally then would be enough since the info would be of little to no use for the community contributors. If, however, folks external to the WinUI team can also have these permissions then this case should be covered in the documentation.)

Thanks for doing this, I think it makes sense to keep both. The internal path is available to any Microsoft employee so there would be a number for 1st party customer teams that could appreciate that documentation.

@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Felix-Dev
Copy link
Contributor Author

In that case I will add a section for MS employees (using the previous images) and another section for external contributors (using the new images introduced with this PR).

@StephenLPeters StephenLPeters added area-TestInfrastructure Issue in the test infrastructure (e.g. in Helix scripts) documentation An issue with existing documentation or a request for documenation of a new topic team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Jun 15, 2020
@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@StephenLPeters
Copy link
Contributor

as per #2725 we are changing Master to VerificationFile and should probably update the terminology here as well.

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.

🕐

@StephenLPeters
Copy link
Contributor

@Felix-Dev I think this is ready to go in once its merged with @chingucoding change that went in yesterday.

@Felix-Dev
Copy link
Contributor Author

@chingucoding @StephenLPeters Am I correct when I assume the folder in the downloadable drop archive from Azure Pipelines containing the updated verification files has been renamed from "VisualTreeMasters" to "UpdatedVisualTreeVerificationFiles"?

There's also this image which needs to be updated then: https://github.com/microsoft/microsoft-ui-xaml/blob/master/docs/images/masters_folder.png
As I don't have permission to download the new verification files directly in Azure Pipelines, someone from the team should probably share the updated image with me here and I can then include it in my PR.

@marcelwgn
Copy link
Collaborator

@chingucoding @ StephenLPeters Am I correct when I assume the folder in the downloadable drop archive from Azure Pipelines containing the updated verification files has been renamed from "VisualTreeMasters" to "UpdatedVisualTreeVerificationFiles"?

Yes I think that that would be the new folder name.

@StephenLPeters
Copy link
Contributor

DropPublishLocation
linksToHelix
VisualVerificationLink

@StephenLPeters
Copy link
Contributor

The UI has changed since that screenshot was taken =/ I've put images for how you get there from an employee perspective. @Felix-Dev if you don't want your name in the doc picture let me know and I'll find a different PR to take it from.

@Felix-Dev
Copy link
Contributor Author

Felix-Dev commented Jul 6, 2020

@StephenLPeters Thanks for sharing the updated screenshots. No, as far as I'm concerned, you don't have to find a different screenshot. At most I will edit out my name and replace it with [PR author] as done initially in this PR. That said...I don't think I have an issue with my github handle listed there.

Will include the new images in this PR soon!

Edit 2: Nevermind.

@Felix-Dev
Copy link
Contributor Author

I'm not yet done with updating this PR.

@Felix-Dev
Copy link
Contributor Author

Felix-Dev commented Jul 6, 2020

@StephenLPeters Done. Please take a look.

Edit: Also had to update the download steps for external contributors as they now also need to open the LinksToHelixTestFiles.html file.

@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

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

@Felix-Dev can you merge master once more to pick up the heap limit fix keith checked in?

@Felix-Dev
Copy link
Contributor Author

@StephenLPeters Done.

@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ranjeshj ranjeshj merged commit 941aa42 into microsoft:master Jul 13, 2020
@Felix-Dev Felix-Dev deleted the user/Felix-Dev/devdoc-update branch July 13, 2020 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-TestInfrastructure Issue in the test infrastructure (e.g. in Helix scripts) documentation An issue with existing documentation or a request for documenation of a new topic team-Controls Issue for the Controls team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants