-
Notifications
You must be signed in to change notification settings - Fork 60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace terminal UI with a more readable logger when src actions exec is run in verbose mode #140
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, this shortens that waay to bloated function into more readable bits 🌟
diffSupportsNoDereference, err = diffSupportsFlag(ctx, "--no-dereference") | ||
if err != nil { | ||
return err | ||
} | ||
if !diffSupportsNoDereference { | ||
logger.Warnf("Local installation of 'diff' doesn't support '%s' flag. Consider upgrading to GNU diff >=3.7.\n", "--no-dereference") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3.3 I think. For some reason the suggestion
button is disabled so I cannot make one 🤦
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You think this is necessary? Newest version is 3.7
, I think we should recommend upgrading to that :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup definitely makes sense to have people use always the most recent version, I was thinking it might confuse people when they are on some OS that doesn't have 3.7 in their package lists that they cannot use src-cli, but it might be unreasonable to account for that 😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's stay with this for now, since I also want to explore some other options (to fix #137) and this might be invalidated soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure :)
… is run in verbose mode (#140) * Change verbose mode of action exec and not show UI * Introduce actionLogger and print everything through it in verbose mode * Send generic system messages to actionLogger
When
src action exec
previously ran in verbose-mode with the-v
flag, it looked like this:With the changes in this PR it looks like this:
Please note: I didn't focus on the details (such as colors, exact words in message, etc). The goal was to untangle the logging from the
actionExecutor
and the terminal UI, while keeping the behavior in place:-keep-logs
flag is given or the repo didn't produce an errorThis fixes #119