Skip to content

Conversation

@chrisd8088
Copy link
Member

@chrisd8088 chrisd8088 commented Feb 12, 2025

In PR #68 the commands package was updated with several logging functions such as Panic(), Print(), and Debug(). The last of these only outputs a log message if the Debugging variable in the same package is set to true, which at the time was controlled by the setupDebugging() function. If the user supplied the --debug option to a given Git LFS command, setupDebugging() function would set the Debugging variable to true, and any calls to Debug() would then generate log messages.

However, in PR #84 the setupDebugging() function was removed, along with a lot of the earlier command-line parsing and setup code, and replaced with the Cobra library from the spf13/cobra project. Since then it has not been possible to set the Debugging variable to a value other than false, so the remaining references to the Debug() function will never generate log messages.

We therefore remove the Debugging variable and the Debug() function, and replace any calls to that function with ones to our standard trace logging function.

This change should help reduce the number of non-constant format strings we need to revise before adopting Go 1.24, as has been reported in #5968.

In PR git-lfs#68 the "commands" package was updated with several logging
functions such as Panic(), Print(), and Debug().  The last of these
only outputs a log message if the Debugging variable in the same
package is set to "true", which at the time was controlled by the
setupDebugging() function.  If the user supplied the --debug option
to a given Git LFS command, setupDebugging() function would set the
Debugging variable to "true", and any calls to Debug() would then
generate log messages.

However, the setupDebugging() function was removed, along with a
lot of the earlier command-line parsing and setup code, in PR git-lfs#84,
and replaced with the Cobra library from the spf13/cobra project.
Since then it has not been possible to set the Debugging variable
to a value other than "false", so the remaining references to the
Debug() function will never generate log messages.

We therefore remove the Debugging variable and the Debug() function,
and replace any calls to that function with ones to our standard
trace logging function.
@chrisd8088 chrisd8088 requested a review from a team as a code owner February 12, 2025 21:37
Panic(err, tr.Tr.Get("Unable to move %s to %s", tmpfile, mediafile))
}

Debug(tr.Tr.Get("Writing %s", mediafile))
Copy link
Member

Choose a reason for hiding this comment

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

We don't want to translate these messages because it is debug output?

Copy link
Member Author

@chrisd8088 chrisd8088 Feb 14, 2025

Choose a reason for hiding this comment

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

Exactly. We don't offer localization of trace log messages (as per a few random examples from the transfer queue, just for illustrative purposes.)

@chrisd8088 chrisd8088 merged commit 1735acc into git-lfs:main Feb 14, 2025
10 checks passed
@chrisd8088 chrisd8088 deleted the replace-debugging-mode branch February 14, 2025 15:58
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.

2 participants