Store bug reports in separate directories#3150
Conversation
dbkr
left a comment
There was a problem hiding this comment.
Admittedly I'm not familiar with Go and its norms, but I find this quite difficult to understand, especially lines like if _, err := os.Stat(fpath); err == nil which has an awful lot going on for a single line.
|
|
||
| func gzipAndSave(data []byte, fpath string) error { | ||
| fpath = filepath.Join("bugs", fpath) | ||
| func gzipAndSave(data []byte, dirname, fpath string) error { |
There was a problem hiding this comment.
Unclear about what is the norm with Go, but does dirname not get a type?
There was a problem hiding this comment.
It does. Go allows you to omit types on function declarations if subsequent ones are the same type. E.g:
func foo(a int, b, c string) {
// ...
}Is the same as:
func foo(a int, b string, c string) {
// ...
}| } | ||
| for i, log := range p.Logs { | ||
| if err := gzipAndSave([]byte(log.Lines), fmt.Sprintf("%s-%d.log.gz", prefix, i)); err != nil { | ||
| if err := gzipAndSave([]byte(log.Lines), prefix, fmt.Sprintf("logs-%d.log.gz", i)); err != nil { |
There was a problem hiding this comment.
What's the reason for saving each one as a separate file rather than concatenating them all together (perhaps even with a separator)? Do the logs have timestamps?
There was a problem hiding this comment.
Also I would suggest some level of slightly deeper nesting, like at least YYYY-MM-DD/HHMMSS/ otherwise you're going to find that before long, the directory kills your terminal if you type ls.
There was a problem hiding this comment.
(and you already know my view on using timestamps as IDs, and not particularly fine-grained timestamps at that)
There was a problem hiding this comment.
What's the reason for saving each one as a separate file rather than concatenating them all together (perhaps even with a separator)? Do the logs have timestamps?
The logs have timestamps. We cannot concatenate all the files together as that breaks causality. The number of files corresponds to the number of sessions where a session is effectively a browser tab. vdh wants to be able to see logs for each browser tab side-by-side rather than muxxed together. A session's position in the list of files is governed by the most-recent log timestamp.
Also I would suggest some level of slightly deeper nesting, like at least YYYY-MM-DD/HHMMSS/ otherwise you're going to find that before long, the directory kills your terminal if you type ls.
Good idea 👍
(and you already know my view on using timestamps as IDs, and not particularly fine-grained timestamps at that)
Well given we are now representing timestamps as directories, I don't mind having a monotonically increasing integer as part of the filename, though it might be weird to see 2 bug reports in the same directory?
There was a problem hiding this comment.
Yep, I'm assuming you'd put the monotonically increasing integer in the last directory name. Good point about multiple sessions.
|
Now nested as |
And remove the long filenames now that there is a directory above them.