-
Notifications
You must be signed in to change notification settings - Fork 247
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
fix(dotnet/analyzer): remove dependency on Runtime #927
Conversation
Remove the dependency from the Amazon.JSII.Analyzers package to the Amazon.JSII.Runtime package - as that dependency was not used and turned out to be a source of problems. Also - pins the version of the runtimes to the exact version (this could later be extended to a range).
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
<PackageReference Include="Amazon.JSII.Runtime" Version="$(JsiiVersion)" /> | ||
<PackageReference Include="Microsoft.CodeAnalysis" Version="3.2.0" /> | ||
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="3.2.0" /> | ||
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="3.2.0" PrivateAssets="all" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why this needs to be PrivateAssets="all".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is part of the (Microsoft-authored) tutorial I have found... So I fixed it up to match... I reckon the reason is that the compiler actually comes with it's own and it's better to not pollute, but I could be wrong.
The reference code from Microsoft's tutorial is here:
...CalculatorPackageId.BasePackageId/Amazon.JSII.Tests.CalculatorPackageId.BasePackageId.csproj
Outdated
Show resolved
Hide resolved
I don't feel qualified to comment on this PR at all |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it! |
Remove the dependency from the Amazon.JSII.Analyzers package to the
Amazon.JSII.Runtime package - as that dependency was not used and
turned out to be a source of problems.
Also - pins the version of the runtimes to the exact version (this
could later be extended to a range).
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.