Skip to content

Conversation

@wiwei
Copy link
Contributor

@wiwei wiwei commented Aug 12, 2019

In looking into #5568, I found that the existing MRTK files code would look for specific folder names that MRTK gets put into (as in, containing folder names). It had very specific behaviors with respect to NuGetForUnity or consumption as .unitypackage - if you had consumed the code some other way (for example, used a manual nuget restore step separately from NuGetForUnity) this wouldn't work out, because the naming of the folders would be different (i.e. MRTK/) instead of (Microsoft.MixedReality.Toolkit/*).

This change works around all that by introducing the concept of "sentinel" files, which are empty files that are named in a specific way and uniquely identify the root of an MRTK module/subfolder. Thus, when the MRTK files code finds a file called MRTK.SDK.sentinel, it knows that that folder is the root of the SDK MRTK subfolder, and it doesn't matter what name the root folder (or folders going up the root) are.

wiwei added 2 commits August 9, 2019 15:13
…to three directories deep.

In looking into microsoft#5568, I found that the existing MRTK files code would look for specific folder names that MRTK gets put into (as in, containing folder names). It had very specific behaviors with respect to NuGetForUnity or consumption as .unitypackage - if you had consumed the code some other way (for example, used a manual nuget restore step separately from NuGetForUnity) this wouldn't work out, because the naming of the folders would be different (i.e. MRTK/*) instead of (Microsoft.MixedReality.Toolkit*/*).

This change works around all that by introducing the concept of "sentinel" files, which are empty files that are named in a specific way and uniquely identify the root of an MRTK module/subfolder. Thus, when the MRTK files code finds a file called MRTK.SDK.sentinel, it knows that that folder is the root of the SDK MRTK subfolder, and it doesn't matter what name the root folder (or folders going up the root) are.
@wiwei wiwei requested review from a user and andreiborodin August 12, 2019 15:41
@wiwei
Copy link
Contributor Author

wiwei commented Aug 12, 2019

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link

@andreiborodin andreiborodin left a comment

Choose a reason for hiding this comment

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

Great idea, awesome change, requested some minor updates to the post process function.

Per PR comments, we should just run the regex directly on the asset filename, instead of truncating to folder, and then looking at all files within it.
@wiwei
Copy link
Contributor Author

wiwei commented Aug 14, 2019

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@wiwei
Copy link
Contributor Author

wiwei commented Aug 14, 2019

/azp run mrtk_docs

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@wiwei wiwei merged commit 585610e into microsoft:mrtk_development Aug 14, 2019
wiwei added a commit to wiwei/MixedRealityToolkit-Unity that referenced this pull request Aug 23, 2019
Update the MRTK to support being placed in arbitrary folder names up to three directories deep.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants