[ 
https://issues.apache.org/jira/browse/AVRO-3402?focusedWorklogId=727326&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-727326
 ]

ASF GitHub Bot logged work on AVRO-3402:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 15/Feb/22 18:40
            Start Date: 15/Feb/22 18:40
    Worklog Time Spent: 10m 
      Work Description: zcsizmadia commented on a change in pull request #1549:
URL: https://github.com/apache/avro/pull/1549#discussion_r806240577



##########
File path: lang/csharp/src/apache/main/Generic/GenericWriter.cs
##########
@@ -29,59 +30,63 @@ namespace Avro.Generic
     public delegate void Writer<T>(T t);
 
     /// <summary>
-    /// A typesafe wrapper around DefaultWriter. While a specific object of 
DefaultWriter
+    /// A type-safe wrapper around DefaultWriter. While a specific object of 
DefaultWriter
     /// allows the client to serialize a generic type, an object of this class 
allows
     /// only a single type of object to be serialized through it.
     /// </summary>
     /// <typeparam name="T">The type of object to be serialized.</typeparam>
+    /// <seealso cref="DatumWriter&lt;T&gt;" />
     public class GenericWriter<T> : DatumWriter<T>
     {
-        private readonly DefaultWriter writer;
+        private readonly DefaultWriter _writer;
 
         /// <summary>
-        /// Initializes a new instance of the <see cref="GenericWriter{T}"/> 
class.
+        /// Initializes a new instance of the <see cref="GenericWriter{T}" /> 
class.
         /// </summary>
         /// <param name="schema">Schema to use when writing.</param>
         public GenericWriter(Schema schema) : this(new DefaultWriter(schema))
         {
-
         }
 
         /// <inheritdoc/>
-        public Schema Schema { get { return writer.Schema; } }
+        public Schema Schema
+        { get { return _writer.Schema; } }

Review comment:
       Maybe just "public Schema Schema => _writer.Schema;``, since it is a 
getter only?

##########
File path: lang/csharp/src/apache/main/Generic/GenericWriter.cs
##########
@@ -29,59 +30,63 @@ namespace Avro.Generic
     public delegate void Writer<T>(T t);
 
     /// <summary>
-    /// A typesafe wrapper around DefaultWriter. While a specific object of 
DefaultWriter
+    /// A type-safe wrapper around DefaultWriter. While a specific object of 
DefaultWriter
     /// allows the client to serialize a generic type, an object of this class 
allows
     /// only a single type of object to be serialized through it.
     /// </summary>
     /// <typeparam name="T">The type of object to be serialized.</typeparam>
+    /// <seealso cref="DatumWriter&lt;T&gt;" />
     public class GenericWriter<T> : DatumWriter<T>
     {
-        private readonly DefaultWriter writer;
+        private readonly DefaultWriter _writer;
 
         /// <summary>
-        /// Initializes a new instance of the <see cref="GenericWriter{T}"/> 
class.
+        /// Initializes a new instance of the <see cref="GenericWriter{T}" /> 
class.
         /// </summary>
         /// <param name="schema">Schema to use when writing.</param>
         public GenericWriter(Schema schema) : this(new DefaultWriter(schema))
         {
-
         }
 
         /// <inheritdoc/>
-        public Schema Schema { get { return writer.Schema; } }
+        public Schema Schema
+        { get { return _writer.Schema; } }

Review comment:
       Maybe just `public Schema Schema => _writer.Schema;`, since it is a 
getter only?

##########
File path: lang/csharp/src/apache/main/Generic/GenericWriter.cs
##########
@@ -29,59 +30,63 @@ namespace Avro.Generic
     public delegate void Writer<T>(T t);
 
     /// <summary>
-    /// A typesafe wrapper around DefaultWriter. While a specific object of 
DefaultWriter
+    /// A type-safe wrapper around DefaultWriter. While a specific object of 
DefaultWriter
     /// allows the client to serialize a generic type, an object of this class 
allows
     /// only a single type of object to be serialized through it.
     /// </summary>
     /// <typeparam name="T">The type of object to be serialized.</typeparam>
+    /// <seealso cref="DatumWriter&lt;T&gt;" />
     public class GenericWriter<T> : DatumWriter<T>
     {
-        private readonly DefaultWriter writer;
+        private readonly DefaultWriter _writer;
 
         /// <summary>
-        /// Initializes a new instance of the <see cref="GenericWriter{T}"/> 
class.
+        /// Initializes a new instance of the <see cref="GenericWriter{T}" /> 
class.
         /// </summary>
         /// <param name="schema">Schema to use when writing.</param>
         public GenericWriter(Schema schema) : this(new DefaultWriter(schema))
         {
-
         }
 
         /// <inheritdoc/>
-        public Schema Schema { get { return writer.Schema; } }
+        public Schema Schema
+        { get { return _writer.Schema; } }

Review comment:
       It is most likely because of `dotnet_diagnostic.IDE0021` - 
`dotnet_diagnostic.IDE002x`. Many settings can be controlled by  editorconfig 
and the ruleset file. However the ruleset file will have higher precedence. IMO 
settings which are controllable by ruleset files should be in the ruleset file 
only, because it is easier to track back the actual warning generated by the 
compiler. editorconfig should have only the settings which generate the 'Fix 
formatting' or other kind of umbrella warnings.

##########
File path: lang/csharp/src/apache/main/Generic/GenericWriter.cs
##########
@@ -29,59 +30,63 @@ namespace Avro.Generic
     public delegate void Writer<T>(T t);
 
     /// <summary>
-    /// A typesafe wrapper around DefaultWriter. While a specific object of 
DefaultWriter
+    /// A type-safe wrapper around DefaultWriter. While a specific object of 
DefaultWriter
     /// allows the client to serialize a generic type, an object of this class 
allows
     /// only a single type of object to be serialized through it.
     /// </summary>
     /// <typeparam name="T">The type of object to be serialized.</typeparam>
+    /// <seealso cref="DatumWriter&lt;T&gt;" />
     public class GenericWriter<T> : DatumWriter<T>
     {
-        private readonly DefaultWriter writer;
+        private readonly DefaultWriter _writer;
 
         /// <summary>
-        /// Initializes a new instance of the <see cref="GenericWriter{T}"/> 
class.
+        /// Initializes a new instance of the <see cref="GenericWriter{T}" /> 
class.
         /// </summary>
         /// <param name="schema">Schema to use when writing.</param>
         public GenericWriter(Schema schema) : this(new DefaultWriter(schema))
         {
-
         }
 
         /// <inheritdoc/>
-        public Schema Schema { get { return writer.Schema; } }
+        public Schema Schema
+        { get { return _writer.Schema; } }

Review comment:
       It is most likely because of `dotnet_diagnostic.IDE0021` - 
`dotnet_diagnostic.IDE002x` are set silent in the ruleset file. Many settings 
can be controlled by  editorconfig and the ruleset file. However the ruleset 
file will have higher precedence. IMO settings which are controllable by 
ruleset files should be in the ruleset file only, because it is easier to track 
back the actual warning generated by the compiler. editorconfig should have 
only the settings which generate the 'Fix formatting' or other kind of umbrella 
warnings.

##########
File path: lang/csharp/src/apache/main/Avro.main.csproj
##########
@@ -49,7 +49,21 @@
 
   <!-- See lang/csharp/README.md for tool and library dependency update 
strategy -->
   <ItemGroup>
+    <PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.0.1" 
/>

Review comment:
       plz dont use direct versioning

##########
File path: lang/csharp/src/apache/main/Avro.main.csproj
##########
@@ -49,7 +49,21 @@
 
   <!-- See lang/csharp/README.md for tool and library dependency update 
strategy -->
   <ItemGroup>
+    <PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.0.1" 
/>
+    <PackageReference Include="Microsoft.CodeAnalysis.CSharp.CodeStyle" 
Version="4.0.1">

Review comment:
       Code styling related package referncing is handled by common.props, to 
make sure that every projects inherit the same codestyling

##########
File path: lang/csharp/CodeMaid.config
##########
@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="utf-8"?>

Review comment:
       Do we need this file? It seems user specific




-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 727326)
    Time Spent: 5h 40m  (was: 5.5h)

> Update Avro.Main.GenericWriter to meet style guidelines
> -------------------------------------------------------
>
>                 Key: AVRO-3402
>                 URL: https://issues.apache.org/jira/browse/AVRO-3402
>             Project: Apache Avro
>          Issue Type: Sub-task
>          Components: csharp
>    Affects Versions: 1.11.0
>            Reporter: Kyle Schoonover
>            Priority: Minor
>              Labels: pull-request-available
>          Time Spent: 5h 40m
>  Remaining Estimate: 0h
>
> Update GenericWriter to meet the style guidelines outlined in editorconfig



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

Reply via email to