CurtHagenlocher commented on code in PR #3218:
URL: https://github.com/apache/arrow-adbc/pull/3218#discussion_r2246271268


##########
csharp/src/Apache.Arrow.Adbc/Tracing/ActivityTrace.cs:
##########
@@ -33,8 +33,11 @@ public sealed class ActivityTrace : IDisposable
         /// Constructs a new <see cref="ActivityTrace"/> object. If <paramref 
name="activitySourceName"/> is set, it provides the
         /// activity source name, otherwise the current assembly name is used 
as the activity source name.
         /// </summary>
-        /// <param name="activitySourceName"></param>
-        public ActivityTrace(string? activitySourceName = default, string? 
activitySourceVersion = default, string? traceParent = default)
+        /// <param name="activitySourceName">The name of the ActivitySource 
object</param>
+        /// <param name="activitySourceVersion">The version of the component 
publishing the tracing info.</param>
+        /// <param name="traceParent">The trace parent context, which is used 
to link the activity to a distributed trace.</param>
+        /// <param name="tags">The tags associated with the activity.</param>
+        public ActivityTrace(string? activitySourceName = default, string? 
activitySourceVersion = default, string? traceParent = default, 
IEnumerable<KeyValuePair<string, object?>>? tags = default)

Review Comment:
   This is a binary breaking change. Was this class already part of the last 
release? If we had "declared 1.0" and already included this class in a public 
release then we'd want to add an overload instead of changing the signature. 
(As we are not yet at 1.0, I think it's probably okay not to worry about binary 
compatibility in this case.)



-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to