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

New project format and netstandard for analyzer project #19

Merged
merged 2 commits into from
Mar 19, 2018

Conversation

JohanLarsson
Copy link
Contributor

No description provided.

@mikkelbu mikkelbu changed the title New project format and netstandard for analyzer project. New project format and netstandard for analyzer project Mar 18, 2018
@mikkelbu
Copy link
Member

Hi @JohanLarsson thanks for the contribution. I've been going a bit back and forth on whether to use the new project format (see e.g. #16), but I think that I've come to the conclusion that the new format is the way to go. Hopefully, I'll have time to look at this PR tomorrow evening, as the last week was completely packed.

@JohanLarsson
Copy link
Contributor Author

If nothing else the new format diffs nicer. I moved to new format for all my analyzer projects and it has worked ok. Only issue really is the vsix that needs a few hax.
I have not tested if dotnet pack detects and packs an analyzer project correctly, would be nice if it did, then the nuget stuff could be removed.

@jnm2
Copy link
Contributor

jnm2 commented Mar 19, 2018

@JohanLarsson I've been using dotnet pack which works great with this. Example at #16 (comment).

Copy link
Member

@mikkelbu mikkelbu left a comment

Choose a reason for hiding this comment

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

Only a couple of comments about the PR 😄 .

(My plan is to have a subsequent sweep over the project files to remove unnecessary elements and add missing ones, but I think it is fine to do this incrementally).

<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<MinimumVisualStudioVersion>11.0</MinimumVisualStudioVersion>
Copy link
Member

Choose a reason for hiding this comment

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

Should this still be in the project-file?

Copy link
Contributor Author

@JohanLarsson JohanLarsson Mar 19, 2018

Choose a reason for hiding this comment

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

Not sure, that is why I left it there, came from the old project.

Copy link
Contributor

Choose a reason for hiding this comment

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

This file will only open in VS2017 or later, so it could be removed.

<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
<Project Sdk="Microsoft.NET.Sdk">
<Import Project="..\packages\NUnit.3.10.1\build\NUnit.props" Condition="Exists('..\packages\NUnit.3.10.1\build\NUnit.props')" />
Copy link
Member

Choose a reason for hiding this comment

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

I had to remove this line and the Target element below (with the name EnsureNuGetPackageBuildImports) otherwise I got complaints about "This project references NuGet package(s) that are missing on this computer. ...The missing file is ..\packages\NUnit.3.10.1\build\NUnit.props." when compiling.

Copy link
Member

@mikkelbu mikkelbu left a comment

Choose a reason for hiding this comment

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

Once more, thanks for the contribution 👍 . This definitely removes a lot of noise.

@mikkelbu mikkelbu merged commit f8fd2c0 into nunit:master Mar 19, 2018
@mikkelbu mikkelbu added this to the Release 0.2 milestone Apr 13, 2020
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.

3 participants