swellaby / rusty-hook

git hook manager, geared toward Rust projects
MIT License
206 stars 11 forks source link

Improve Logging Output #102

Closed calebcartwright closed 3 years ago

calebcartwright commented 4 years ago

Description

Let's update the logging output to clearly differentiate the informational/status messages that rusty-hook writes from hook script output.

Value

Currently the output emitted by rusty-hook is just a stdout line that can often bleed into the blobs of output caused by running the hook scripts (for example in cargo subcommand outputs)


As far as implementation, I think something as simple as a message prefix and/or some new lines would be helpful. For example, with a cargo test script for pre-commit the terminal output would be something like this:

Found configured hook: pre-commit
Running command: cargo test
   Compiling autocfg v1.0.0
   Compiling serde v1.0.106
   Compiling fsio v0.1.2
   Compiling nias v0.5.0
............

maybe we could tweak that to:

[rusty-hook] Found configured hook: pre-commit
[rusty-hook] Running command: cargo test

   Compiling autocfg v1.0.0
   Compiling serde v1.0.106
   Compiling fsio v0.1.2
   Compiling nias v0.5.0
............

It may be helpful to also include some kind of footer message when the hook is successful too (we currently only do so when the hook fails)

Mastermindaxe commented 3 years ago

We could use the log crate or the env_log crate to achieve this. This would also help to set different logging levels (ERROR, WARN, DEBUG and TRACE).

It's not clear for me which crate we should be using as this is somewhat a library but also a binary. What are your thoughts?

calebcartwright commented 3 years ago

We could use the log crate or the env_log crate to achieve this.

True, though I'd really prefer to avoid adding more to the dependency tree, and I don't think we really need the extra features that crates like those bring.

It's not clear for me which crate we should be using as this is somewhat a library but also a binary.

rusty-hook is just a binary. There's technically a lib target that exists, but it's empty and only exists so folks can add as a dev dependency to automate the setup process.

Feel free to open a PR with any suggested solution, though do keep in mind that the ability to test that the configured hook output is logged is really important given the core scope/purpose of rusty-hook

Mastermindaxe commented 3 years ago

I will tackle this as part of the Hacktoberfest if you don't mind.

though do keep in mind that the ability to test that the configured hook output is logged is really important given the core scope/purpose of rusty-hook

What exactly do you mean with test that the configure hook output is logged. As in test by hand so we see that it actually logs or test as in test with some form of unit test?

calebcartwright commented 3 years ago

As in test by hand so we see that it actually logs or test as in test with some form of unit test?

Automated tests in general, specifically unit tests. The current unit tests, for example the one below, can be pretty simple because of the way the logging function is currently used (and could even be extended to verify content)

https://github.com/swellaby/rusty-hook/blob/39fbca3b90c6f37bfdacd23bc4d25b9a0da554f0/src/rusty_hook_test.rs#L140-L163

Mastermindaxe commented 3 years ago

Taking this on over the next few days!