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

Enable Power Tools for VS 2017 and VS 2015 (also removes support for VS 2010) #154

Closed
wants to merge 3 commits into from

Conversation

ErikEJ
Copy link
Contributor

@ErikEJ ErikEJ commented Dec 15, 2016

Addresses #152

This PR shows what is required to move PowerTools to VS 2017 and VSIX 2/3.

Currently pressing F5 to build and deploy in VS 2015 fails, but it is possible to build a release VSIX using the following command line:

 msbuild PowerTools.sln  /p:configuration=Release /p:DeployExtension=false /v:m

Info about some of the changes is available here: https://docs.microsoft.com/da-dk/visualstudio/extensibility/how-to-migrate-extensibility-projects-to-visual-studio-2017

I have successfully installed the built release VSIX and tested with VS 2017 RC, see screenshots below:
powertools1
powertools2

@lajones
Copy link
Contributor

lajones commented Dec 15, 2016

@ErikEJ Erik, that's really great and thanks very much. However at the same time we need to keep providing the old VSIX format so that people working on older versions of VS will be able to install it there. We're not quite sure how we plan to do that yet (a lot of people are already off for the holidays). I'll discuss with @divega and we'll get back to you.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Dec 15, 2016

@lajones The produced VSIX is installable on VS 2012 and higher! (VSIX 3 is "backwards compatible") - it just contains a couple of new json files in addition to the files in a VSIX2 package - I attach my test one for your information or even test? (remove the .zip extension)

EFPowerTools.vsix.zip

@lajones
Copy link
Contributor

lajones commented Dec 15, 2016

Cool. I did not think that would be the case. Let me have a chat with @divega and we'll let you know.

<Name>Entity Framework Power Tools Beta 5</Name>
<Author>Microsoft</Author>
<Version>0.9.0.0</Version>
<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
Contributor

Choose a reason for hiding this comment

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

Is this correct? The Version is 2.0.0? I would have thought it would be 3.0.0.

Copy link
Contributor

Choose a reason for hiding this comment

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

I know @bricelam tried v2 manifest on Dev15 and it didn't work. Maybe there is something else we were doing?

Copy link
Contributor Author

@ErikEJ ErikEJ Dec 15, 2016

Choose a reason for hiding this comment

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

@lajones Yes, it is correct, and trust me it works (as demonstrated) - there is no 3.0.0 - the main point (as stated earlier) is that the VSIX 3 file contains two additional json files (provided by using the Dev15 build nugte package), and that the manifest specifies prerequisites... You can look inside the VSIX to see what I mean.

@divega @bricelam You need the build nuget package (which injects the two additionla json files) , and you must specify at least one prerequisite in the manifest xml file

Copy link
Contributor

Choose a reason for hiding this comment

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

@ErikEJ Ok, I am all for trying! I guess I had misunderstood how it was supposed to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@divega It is quite confusing - maybe it should have been called VSIX2+ or VSIX 2.5 😄

@@ -205,6 +207,14 @@
<Import Project="$(MSBuildBinPath)\Microsoft.CSharp.targets" />
<Import Project="$(VSToolsPath)\VSSDK\Microsoft.VsSDK.targets" Condition="'$(VSToolsPath)' != ''" />
<Import Project="$(SolutionDir)\.nuget\nuget.targets" />
<Target Name="EnsureNuGetPackageBuildImports" BeforeTargets="PrepareForBuild">
Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't these added elements only used in the no longer recommended "msbuild-ingrated package restore". Can we move on to automatic? See https://docs.nuget.org/ndocs/consume-packages/package-restore#migrating-to-automatic-restore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure it is related...

Copy link
Contributor

Choose a reason for hiding this comment

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

Definitively this looks like PowerTools uses the old style package restore. I don't really feel strongly that we need to change it though.

<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">
<Metadata>
<Identity Id="2b119c79-9836-46e2-b5ed-eb766cebbf7c" Version="0.9.1.0" Language="en-US" Publisher="Microsoft" />
<DisplayName>Entity Framework Power Tools Beta 5</DisplayName>
Copy link
Contributor

Choose a reason for hiding this comment

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

The screenshots made me think about rebranding/bumping the version. I think these should be called the Entity Framework 6 Power Tools, probably beta 6. Thoughts? I am sure if this is the only string that needs updated BTW.

Copy link
Contributor Author

@ErikEJ ErikEJ Dec 16, 2016

Choose a reason for hiding this comment

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

Sounds like a good idea to change the name, but why beta 6, the current published package is "beta 4" ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I missed that we had already bumped the version 😄

@@ -14,35 +13,23 @@ When right-clicking on a file containing a derived DbContext class, the followin
1) View Entity Data Model XML - Displays the EDMX XML representing the underlying Code First model.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: is this is in sync with the options actually displayed? See https://github.com/aspnet/EntityFramework6/blob/master/src/PowerTools/DbContextPackage.vsct

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at this page, maybe the short description should just be changed to:
"Preview of useful design-time features for DbContext."

(The description below on that page is entered manually, and is independent of the contents on the VSIX package)

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@divega divega left a comment

Choose a reason for hiding this comment

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

LGTM. Just a few comments.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Dec 16, 2016

@divega Let me know what concrete action you would like me to take on your comments ? 😄

- change name to: Entity Framework 6 Power Tools
- streamline/standardize decriptions
- fixed wrong version displayed in VS About dialog
Copy link
Contributor

@divega divega left a comment

Choose a reason for hiding this comment

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

Looks good to me. @lajones or @bricelam can merge it? Feel free to review it if you haven't already.

@divega divega assigned lajones and unassigned divega Jan 20, 2017
@divega divega requested a review from bricelam January 20, 2017 20:48
@lajones
Copy link
Contributor

lajones commented Jan 20, 2017

@ErikEJ @divega - you reference the RC2 version of Microsoft.VSSDK.BuildTools in a few places. The release of RC3 is already out and RTW should be coming soon. Can we wait on this, update the references to Microsoft.VSSDK.BuildTools to the RTW version once it's available and then check in? (The rest of it all looks fine to me).

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Jan 22, 2017

@lajones Yes, let us just wait for VS 2017 RTW

Shorten desciription (forced by checks in RTW build tools)
@ErikEJ
Copy link
Contributor Author

ErikEJ commented Feb 3, 2017

@lajones @bricelam Updated to RTW build tools!

@lajones
Copy link
Contributor

lajones commented Feb 3, 2017

@ErikEJ @bricelam Brice should check that the new version numbers are good, but assuming so it all looks good to me. Thanks, once again, Erik for your contributions.

</Vsix>
</Metadata>
<Installation>
<InstallationTarget Id="Microsoft.VisualStudio.Pro" Version="[11.0,16.0)" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we leave this unbounded? [11.0,)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Safer not to, IMHO

<Asset Type="Microsoft.VisualStudio.VsPackage" d:Source="Project" d:ProjectName="%CurrentProject%" Path="|%CurrentProject%;PkgdefProjectOutputGroup|" />
</Assets>
<Prerequisites>
<Prerequisite Id="Microsoft.VisualStudio.Component.EntityFramework" Version="[15.0,16.0)" DisplayName="Entity Framework 6 tools" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Unbounded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Safer not to do it, IMHO

Copy link
Contributor

@bricelam bricelam left a comment

Choose a reason for hiding this comment

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

LGTM

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Feb 21, 2017

@divega @lajones @bricelam :shipit: ???

@lajones
Copy link
Contributor

lajones commented Feb 21, 2017

Sorry. I will do this.

@divega divega closed this Mar 7, 2017
@ErikEJ
Copy link
Contributor Author

ErikEJ commented Mar 7, 2017

@lajones @divega How about removing this obsolete feature? (replaced by latest EF Tools and/or the EF Reverse POCO Template) - happy to do a new PR to that effect

@divega
Copy link
Contributor

divega commented Mar 8, 2017

@ErikEJ Other than making only the minimal necessary changes, the main reason to keep the feature was that some customers had expressed a preference for the code PowerTools generate, e.g. using EntityTypeConfiguration<T> classes. I don't think this is a strong argument though.

Unless we can fix the feature easily, I agree removing it (or even disabling it selectively for VS 2017 and VS 2015) is a valid option.

I would like to hear what @bricelam and @ajcvickers think about this.

@bricelam
Copy link
Contributor

bricelam commented Mar 8, 2017

I've been wanting to get rid of it ever since we productized the feature as part of the designer. For those who prefer it, they'll probably find the EF Reverse POCO Template just as satisfying...

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Mar 8, 2017

@divega I think customers wanting Configuration classes will benefit from EF Reverse POCO, not only does it support this, but you can also switch to DataAnnotations (in addition to 100s of other features, like stored procedure and function mappings)

@divega
Copy link
Contributor

divega commented Mar 8, 2017

Sounds good to me.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Mar 8, 2017

@divega on the other hand, just disabling for vs2015/2017 would cause minimal disruption/breaking changes (as these versions were not supported before) !?

@ajcvickers
Copy link
Member

I think at this point I would opt for removing it completely. It's been obsolete for a long time now and it's one less thing to maintain going forward.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Mar 8, 2017

@ajcvickers @divega PR coming up!

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Mar 20, 2017

I have submitted PR #213 - hope that this can be released at some point in time?

@rudolph2907
Copy link

rudolph2907 commented Jun 23, 2017

@ErikEJ Hi, where can I download the vsix file. I cannot find it.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Jun 23, 2017

@rudolph2907 See my earlier comment in this thread with a link to the file.

@rudolph2907
Copy link

Thanks found it.

@antoni-marasek
Copy link

Hello,
When is it planned for the new version of the EF Power Tools (supporting VS17) to be released to the VS Marketplace?
Thanks and regards,
Antoni

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Jul 5, 2017

@antoni-marasek If you need it now, you can download: #154 (comment)

@CZEMacLeod
Copy link

@ErikEJ @antoni-marasek I'm slightly surprised this isn't being built and pushed to the myget nightly feed along with the nuget package.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Jul 6, 2017

@CZEMacLeod It is not a Nuget package, so it does not belong in a Nuget feed (it is a VSIX - VS extension)

@antoni-marasek
Copy link

@ErikEJ Thank you for your response.

Is the version referenced in #154 stable and ready to be used in the commercial projects or is it still a beta version?

In Visual Studio prior to 2017 (e.g. 2015) Power Tools used to be downloadable from the VS Marketplace (i.e. using the Tools -> Extensions menu item available directly in VS). Is it going to be the case also for VS17 and the new version of the plugin?

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Jul 6, 2017

@antoni-marasek It is still at beta, and a couple of rev-eng features have been removed in the release version- I am sure the team intend to release to the MarketPlace, as stated here: https://blogs.msdn.microsoft.com/dotnet/2017/05/23/announcing-ef-6-2-beta-1/ - not sure what is holding them back, as no public test/beta effort is taking place for the PowerTools @divega ?

@CZEMacLeod
Copy link

@ErikEJ I am well aware that the VSIX is not a NuGet package. However MyGet has a special 'feed' section for vsix packages which can be used in Visual Studio as an extension source.
See https://www.myget.org/vsix and http://docs.myget.org/docs/walkthrough/getting-started-with-vsix
It has had this feature since 2016.
I had already added https://www.myget.org/F/aspnetwebstacknightly/vsix/ as a source for Visual Studio assuming you were pushing your nightly builds of both the nuget package and the vsix package from your CI.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Jul 6, 2017

Sorry, misunderstood... I am not on the EF team, but doing this sounds like a good idea - I do the same for my VS extension: https://github.com/ErikEJ/SqlCeToolbox/wiki/Subscribing-to-latest-%22daily%22-build

@divega
Copy link
Contributor

divega commented Jul 6, 2017

@ErikEJ the main blocker for this the last time we tried was to find a certificate that we could sign the VSIX with which would work across all the supported versions of Visual Studio.

We will try again (when we are not heads down with EF Core 2.0 work), but ultimately we may leave this as source code only that you can build and install in your machine as you did.

Is the version referenced in #154 stable and ready to be used in the commercial projects or is it still a beta version?

We don't have any plans to ever move EF PowerTools out of the beta phase.

@ErikEJ
Copy link
Contributor Author

ErikEJ commented Jul 6, 2017

@divega My extension is not signed - could "someone" else host a "homebuilt" version on MarketPlace? Or on their web site? Or would that break too many rules?

@divega
Copy link
Contributor

divega commented Jul 6, 2017

I don’t know the exact rules for the market place, but we can’t ship something without signing it.

@ErikEJ ErikEJ deleted the issue-152 branch July 10, 2017 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants