Skip to content

Conversation

@GarrettBeatty
Copy link
Contributor

Fixing this warning i saw from copilot

         public bool DownloadFilesConcurrently { get; set; }
 
+        private Logger Logger
+        {
+            get { return Logger.GetLogger(typeof(DownloadDirectoryCommand)); }
Recursive property definition: the Logger property calls Logger.GetLogger() which references itself. This should be _logger field or use a different pattern.

Testing

unit tests pass

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed

License

  • I confirm that this pull request can be released under the Apache 2 license

@GarrettBeatty GarrettBeatty changed the title update logging Fix Logger Recursive property definition Dec 3, 2025
@GarrettBeatty GarrettBeatty marked this pull request as ready for review December 3, 2025 17:08
private long _lastKnownTransferredBytes;

private static Logger Logger
private static Logger LoggerInstance
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason you didn't use private readonly Logger _logger = Logger.GetLogger(...) like you did in some other files?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ill fix this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

"serviceName": "S3",
"type": "patch",
"changeLogMessages": [
"Fix Transfer Utility internal Logger recurisive property definition"
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: recurisive => recursive

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

private long _lastKnownTransferredBytes;

private static Logger Logger
private static Logger LoggerInstance
Copy link
Contributor

Choose a reason for hiding this comment

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

+1

int total = pendingTasks.Count;

Logger.DebugFormat("TaskHelpers.WhenAllOrFirstExceptionAsync: Starting with TotalTasks={0}", total);
Logger.GetLogger(typeof(TaskHelpers)).DebugFormat("TaskHelpers.WhenAllOrFirstExceptionAsync: Starting with TotalTasks={0}", total);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

couldnt declare a non static variable in this class so just did it this way

@GarrettBeatty GarrettBeatty merged commit a93e67b into feature/transfermanager Dec 3, 2025
1 check passed
@GarrettBeatty GarrettBeatty deleted the gcbeatty/loggerfix branch December 3, 2025 18:20
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