sandbox: better log output #43325
sandbox: better log output #43325
Conversation
| puts Sandbox.log_warning_message | ||
| puts logs | ||
| ohai "Sandbox profile:" | ||
| puts @profile.dump |
There was a problem hiding this comment.
Do you find this message useful? If so we can keep it otherwise let's 💀
There was a problem hiding this comment.
It's useful to people who are unfamiliar to homebrew sandbox's rule. Otherwise, it's not as they are almost constant.
There was a problem hiding this comment.
Yeh, I guess I personally find this output kinda noise and hard to follow. Maybe we could add a few newlines or separators in here to split it up a bit if we leave it in?
There was a problem hiding this comment.
Or we remove this. And add it to document, say sandbox.md?
There was a problem hiding this comment.
Yeh, maybe just remove it? A document will get outdated compared to the code.
|
Rewrote PR to address the comments. |
| def self.log_warning_message; <<-EOS.undent | ||
| We use time to filter sandbox log. Therefore, unrelated logs may be recorded in below. | ||
|
|
||
| EOS | ||
| end |
There was a problem hiding this comment.
I think we can probably skip printing this message unless it's causing confusion.
There was a problem hiding this comment.
How about only writing it to log file? Usually there won't be unrelated logs for bot. But on users computer, the chance is high which can cause confusion.
There was a problem hiding this comment.
Yep, if we write it maybe after we print the log then that makes sense to me. Equally if we didn't write it if HOMEBREW_DEVELOPER is set or something? Up to you!
There was a problem hiding this comment.
Finally, I decide to only write this message to the bottom of file.
|
Looks good to me! |
* use syslog filter instead of grep. * output sandbox log to stdout when verbose and failed. * output nothing if sandbox log is empty.
|
👍 |
cc @MikeMcQuaid