Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Fix packaging of startpage and inlinereviews vsix's #1314

Merged
merged 3 commits into from
Nov 13, 2017

Conversation

shana
Copy link
Contributor

@shana shana commented Nov 13, 2017

The build system overhaul removed some key parts of the entries that control what gets packaged into the VSIX so pkgdef files for GitHub.StartPage and GitHub.InlineReviews weren't being included. Additionally, both these projects should not be deployable as VSIXs because they're not standalone extensions (they can be for testing purposes but that should be done separately and manually on a case-by-case basis as needed). The combination of these two things means that default local builds would cause these packages to be deployed to the VS experimental instance and override the loading directory for DLLs, which is Not A Good Thing(tm)

Also take the opportunity to fix the publisher and identity of the InlineReviews package to match our other packages.

Fixes #1310

grokys
grokys previously approved these changes Nov 13, 2017
@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<PackageManifest Version="2.0.0" xmlns="http://schemas.microsoft.com/developer/vsx-schema/2011" xmlns:d="http://schemas.microsoft.com/developer/vsx-schema-design/2011">
Copy link
Collaborator

Choose a reason for hiding this comment

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

This source.extension.vsixmanifest shouldn't be used (we're not creating a .vsix for this project). Can we delete it from this project? The same would apply to the on in GitHub.StartPage.

Am I missing something and there's a reason why they can't be deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The vsix file has an entry exporting the pkgdef. I'm not sure you can kill the manifest and still have the project packaging properly. I think you may have to change the project type, and/or maybe have another project type for the pkgdef-but-no-vsix logic. There used to be a project type for this at some point iirc, but honestly I can't be sure so it would need testing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's what I thought as well, but I tried deleting the .vsixmanifest and it still seemed to compile. Maybe this has changed in a recent VSIX project type update? Dunno.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could have, I wouldn't be surprised. I remember the file being mandatory but if they updated the targets file to skip it since then, then that would make sense.

jcansdale
jcansdale previously approved these changes Nov 13, 2017
Copy link
Collaborator

@jcansdale jcansdale left a comment

Choose a reason for hiding this comment

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

The extension is now installing properly for me. 👍

Just a comment about the .vsixmanifest files.

@shana shana dismissed stale reviews from jcansdale and grokys via ea69dff November 13, 2017 12:10
Copy link
Collaborator

@jcansdale jcansdale left a comment

Choose a reason for hiding this comment

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

I'm all for simplifying the solution configurations. 😄

@jcansdale jcansdale merged commit 9b0e2d5 into master Nov 13, 2017
@jcansdale jcansdale deleted the fixes/what-about-them-pkgdefs branch November 13, 2017 15:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add GitHub option back to start page in VS2017
3 participants