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.

Sign up for GitHub

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

Jump to bottom

Fix two VSIX issues stemming from Reunion -> WindowsAppSDK rename #1192

Merged
merged 7 commits into from
Aug 7, 2021
Merged

Fix two VSIX issues stemming from Reunion -> WindowsAppSDK rename #1192

merged 7 commits into from
Aug 7, 2021

Conversation

DefaultRyan
Copy link
Member

After the rename, it was decided to revert the VSIX id to the original
name so as not to break upgradability and continuity. Unfortunately,
the repositoryId elements in the individual templates weren't updated.

Additionally, the original rename introduced a typo in the XmlPoke build
target responsible for replacing the placeholder package versions with
the actual package versions.

@DefaultRyan
Fix two VSIX issues stemming from Reunion -> WindowsAppSDK rename
c3fdca3 
After the rename, it was decided to revert the VSIX id to the original
name so as not to break upgradability and continuity. Unfortunately,
the repositoryId elements in the individual templates weren't updated.

Additionally, the original rename introduced a typo in the XmlPoke build
target responsible for replacing the placeholder package versions with
the actual package versions.
@ghost ghost added the needs-triage label Aug 6, 2021
@DefaultRyan
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

kythant
kythant reviewed Aug 6, 2021
View reviewed changes
dev/VSIX/Directory.Build.targets
@@ -77,7 +77,7 @@
<OriginalNugetRepositoryId>$(_tempVsTemplateNugetRepositoryId)</OriginalNugetRepositoryId>
<!-- 'ExperimentalNugetRepositoryId' needs to match the value set by the XmlPoke query for
"/ns:PackageManifest/ns:Metadata/ns:Identity/@Id" in Extension\WindowsAppSDK.Extension.csproj -->
<ExperimentalNugetRepositoryId>$(_tempVsTemplateNugetRepositoryId).Experimental</ExperimentalNugetRepositoryId>
<ExperimentalNugetRepositoryId>$(_tempVsTemplateNugetRepositoryId).Preview</ExperimentalNugetRepositoryId>
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this make it so the name of the vsix file now has preview in it instead of experimental?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought that VS used the VSIX Id, not the filename. I'm going to try testing it now.

Copy link
Contributor

Choose a reason for hiding this comment

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

It needs to match the VSIX ID, as I noted in the comment. :)

@evelynwu-msft
Copy link
Contributor

evelynwu-msft commented Aug 6, 2021 •
edited
Loading 

        Query="/ns:VSTemplate/ns:TemplateContent/ns:CustomParameters/ns:CustomParameter[@Name='$WindowsAppSdkNupkgVersion$']/@Value"

This casing also needs to be fixed. #Closed

Refers to: dev/VSIX/Directory.Build.targets:218 in bade632. [](commit_id = bade632, deletion_comment = False)

@evelynwu-msft
Copy link
Contributor

evelynwu-msft commented Aug 6, 2021 •
edited
Loading 

        Query="/ns:VSTemplate/ns:TemplateContent/ns:CustomParameters/ns:CustomParameter[@Name='$WindowsAppSdkFoundationNupkgVersion$']/@Value"

This casing also needs to be fixed. #Closed

Refers to: dev/VSIX/Directory.Build.targets:221 in bade632. [](commit_id = bade632, deletion_comment = False)

@evelynwu-msft
Copy link
Contributor

evelynwu-msft commented Aug 6, 2021 •
edited
Loading 

        Query="/ns:VSTemplate/ns:TemplateContent/ns:CustomParameters/ns:CustomParameter[@Name='$WindowsAppSdkDWriteNupkgVersion$']/@Value"

This casing also needs to be fixed. #Closed

Refers to: dev/VSIX/Directory.Build.targets:224 in bade632. [](commit_id = bade632, deletion_comment = False)

@evelynwu-msft
Copy link
Contributor

evelynwu-msft commented Aug 6, 2021 •
edited
Loading 

        Query="/ns:VSTemplate/ns:TemplateContent/ns:CustomParameters/ns:CustomParameter[@Name='$WindowsAppSdkWinUINupkgVersion$']/@Value"

This casing also needs to be fixed. #Closed

Refers to: dev/VSIX/Directory.Build.targets:227 in bade632. [](commit_id = bade632, deletion_comment = False)

@DefaultRyan
Copy link
Member Author

Fixed the cases mentioned by alwu.

@evelynwu-msft
Copy link
Contributor

evelynwu-msft commented Aug 6, 2021 •
edited
Loading

@DefaultRyan Please cherry-pick fe787c7 (fe787c7) for the change that removes the Single-project MSIX Packaging project templates.

@huichen123
Copy link
Contributor

we want this change in main and experimental. But your change may only for experimental.

In reply to: 894549399

@DefaultRyan
Temporarily remove the Single-project MSIX Packaging project template…
238dc03 
…s from the experimental VSIX

(cherry picked from commit fe787c7)
@DefaultRyan
Copy link
Member Author

we want this change in main and experimental. But your change may only for experimental.

In reply to: 894549399

If it's broken, it's broken. In this case, I don't see much, if any benefit to isolating this change only to the experimental release branch.

@DefaultRyan
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Add missing IXP package to project templates, and use an obvious plac…
18f6a8c 
…eholder as the 'default' version in our source .vstemplate files
@evelynwu-msft
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@DefaultRyan
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

evelynwu-msft
evelynwu-msft approved these changes Aug 7, 2021
View reviewed changes
Copy link
Contributor

@evelynwu-msft evelynwu-msft left a comment

Choose a reason for hiding this comment

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

:shipit:

@kythant kythant merged commit a9089e9 into main Aug 7, 2021
@kythant kythant deleted the user/defaultryan/fix_vsix_rename_issues branch August 7, 2021 08:30
This was referenced Aug 7, 2021
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kythant kythant kythant left review comments

@evelynwu-msft evelynwu-msft evelynwu-msft approved these changes

@DrusTheAxe DrusTheAxe Awaiting requested review from DrusTheAxe

@Scottj1s Scottj1s Awaiting requested review from Scottj1s

@huichen123 huichen123 Awaiting requested review from huichen123

Assignees
No one assigned
Labels
needs-triage
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@DefaultRyan @evelynwu-msft @huichen123 @kythant