taskolib / libgit4cpp

C++ wrapper for libgit2 with limited functionality
https://taskolib.github.io/libgit4cpp/
GNU Lesser General Public License v2.1
1 stars 0 forks source link

Remove debug output #13

Closed Finii closed 6 months ago

Finii commented 7 months ago

The wrapper functions output some error messages to stderr when something happens. Sometimes the error is "expected" but the message is printed nonetheless. For example

We have 2 possibilities...

  1. Keep the error messages but only throw them when they are unexpected. For this we expand the LibGitWrapper to be a bit like a gul14::expected (which is, I just noticed, quite different from std::expected). This is implemented in the PR.
  2. Just drop the error messages and hope all errors are handled by the wrapper users.

Fixes: #5

I'm not sure what we want, 1 or 2 or something else. Please feel free to suggest something different, etc. I know tests for the extension of LibGitWrapper are missing, I wanted to wait until I know we really want that ;-)

Edit: Add last sentense

Finii commented 7 months ago

I would be more comfortable with 2 if we hide the wrappers from our users; at the moment it is in a public header. Then we could argue WE are experts and will never ignore errors and thus do not need the error messages at all.

--- a/include/libgit4cpp/meson.build
+++ b/include/libgit4cpp/meson.build
@@ -4,6 +4,9 @@ public_headers = [
     'GitRepository.h',
     'libgit4cpp.h',
     'LibGitPointer.h',
-    'wrapper_functions.h',
 ]
+
+private_headers = [
+    'wrapper_functions.h',
+]

;-)

Finii commented 7 months ago

Interesting idea. Let's discuss this in the meeting.

Sorry I am at the MTCA workshop 😒

Finii commented 7 months ago

Did you talk about this in the meeting today? Shall I do something or wait till next week's meeting?

Fixed the two issues in the original commits, force push.

Finii commented 6 months ago

Did you talk about this in the meeting today?

Obviously some discussion is needed for the change. That is totally fine. But to get taskolib ready we need some solution for the debug messages on stdout.

So I created a new PR:

That PR has all this PR has, just that it does not store/throw the messages. All other code improvements went over to the new PR. And it additionally renamed GitLibPointer which is imho a misnomer, please see reasoning over there.

If we want the storing/throwing of the messages I can/will add that to #14 or a further new PR after a release or whatever.