blachniet commented on a change in pull request #981:
URL: https://github.com/apache/avro/pull/981#discussion_r525669225
##########
File path: lang/csharp/common.props
##########
@@ -45,6 +45,7 @@
<ItemGroup>
<None Include="$(MSBuildThisFileDirectory)\LICENSE" Pack="true"
Visible="false" PackagePath=""/>
+ <None
Include="$(MSBuildThisFileDirectory)\..\..\doc\src\resources\images\avro-logo.png"
Pack="true" PackagePath=""/>
Review comment:
Could you set `Visible="false"` so this file doesn't show up in the file
tree in Visual Studio (like we did with the `LICENSE` file above)?
```suggestion
<None
Include="$(MSBuildThisFileDirectory)\..\..\doc\src\resources\images\avro-logo.png"
Pack="true" Visible="false" PackagePath=""/>
```
##########
File path: lang/csharp/versions.props
##########
@@ -0,0 +1,33 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ https://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+<Project>
+ <PropertyGroup Label="Package Versions">
+ <MicrosoftBuildFrameworkVersion>16.8.0</MicrosoftBuildFrameworkVersion>
+
<MicrosoftBuildUtilitiesCoreVersion>16.8.0</MicrosoftBuildUtilitiesCoreVersion>
+
<MicrosoftCodeAnalysisFxCopAnalyzersVersion>3.3.1</MicrosoftCodeAnalysisFxCopAnalyzersVersion>
+ <MicrosoftNETTestSdkVersion>16.8.0</MicrosoftNETTestSdkVersion>
+ <NewtonsoftJsonVersion>12.0.3</NewtonsoftJsonVersion>
+ <NUnitVersion>3.12.0</NUnitVersion>
+ <NUnitConsoleRunnerVersion>3.11.1</NUnitConsoleRunnerVersion>
+ <NUnit3TestAdapterVersion>3.17.0</NUnit3TestAdapterVersion>
+ <StyleCopAnalyzersVersion>1.1.118</StyleCopAnalyzersVersion>
+ <SystemCodeDomVersion>5.0.0</SystemCodeDomVersion>
+ <SystemReflectionVersion>4.3.0</SystemReflectionVersion>
+
<SystemReflectionEmitILGenerationVersion>4.7.0</SystemReflectionEmitILGenerationVersion>
+
<SystemReflectionEmitLightweightVersion>4.7.0</SystemReflectionEmitLightweightVersion>
+ </PropertyGroup>
+</Project>
Review comment:
👍 I like the idea of the `versions.props` file. I also like that you
broke this out from the `common.props` since some projects need to reference
things in `versions.props` but not `common.props.
We require users of the libraries to include the following dependencies:
- Avro.main
- Newtonsoft.Json
- System.CodeDom
- System.Reflection
- System.Reflection.Emit.ILGeneration
- System.Reflection.Emit.Lightweight
- Avro.msbuild
- Microsoft.Build.Framework
- Microsoft.Build.Utilities.Core
By updating the versions in our libraries, we require users of the library
to update to a version equal to or greater than the version we reference. For
example, if a user were to reference an older version of Newtonsoft.Json, the
would be forced to update to a newer version before they could use a new
version of the Avro library.
In short, we should only update the version of the dependencies in our
libraries if we absolutely must for functionality that we require. We leave it
up to the users of the library as to whether or not they want the latest and
greatest of a particularly dependency. We're only going to require the bare
minimum.
That said, we should use the latest and greatest in any executables we ship
(i.e. avrogen/Avro.codegen) so that we have the latest security fixes in there.
Does all that make sense?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
