Code review comment for lp:~themue/juju-core/057-tailer
Revision history for this message
| Frank Mueller (themue) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
0e1f616
(Get the code!)
Please take a look.
https:/
File utils/tailer/
https:/
utils/tailer/
number of matching lines
On 2013/12/10 15:14:25, rog wrote:
> s/beginns/begins/
Done.
https:/
utils/tailer/
io.ReadSeeker, writer io.Writer, lines int, filter TailerFilterFunc)
*Tailer {
On 2013/12/10 15:14:25, rog wrote:
> Given that this function is going to be the one that everyone calls, I
think I'd
> name it NewTailer, but do we actually need to expose buffer size and
poll
> interval publicly except to the tailer tests?
Done.
https:/
utils/tailer/
On 2013/12/10 15:14:25, rog wrote:
> This seems rather complex to me - I don't fully trust myself to vet
the logic.
Used it, even if I have the same troubles following your array logic and
the needed debugging. Thankfully the comments helped to follow the idea.
And at least it has less lines. ;) Thx.
https:/
utils/tailer/
On 2013/12/10 15:14:25, rog wrote:
> s/beginnig/
Not needed anymore due to new function.
https:/
utils/tailer/
On 2013/12/10 15:14:25, rog wrote:
> ReadBytes can't return ErrBufferFull.
> I think you want to use ReadSlice.
> Also, if the first call succeeds, we should just return the slice that
we got
> from ReadSlice - that way in the usual case that the line is shorter
than the
> bufio buffer we can avoid any allocation.
Done.
https:/
utils/tailer/
delimiter {
On 2013/12/10 15:14:25, rog wrote:
> This can't happen. (from the docs: "returns err != nil if and only if
the
> returned data does not end in delim")
Done.
https:/
utils/tailer/
On 2013/12/10 15:14:25, rog wrote:
> This is racy. We can't seek from the end because the end might be
constantly
> changing. I think we need to keep track of where we're reading in the
file and
> seek back to the absolute offset of the start of the partial line.
Done.
https:/
utils/tailer/
On 2013/12/10 15:14:25, rog wrote:
> Why?
Yeah, too much automatic filtering here. The receiver of the data should
decide. Removed it.
https:/
utils/tailer/
On 2013/12/10 15:14:25, rog wrote:
> Consider trimming \r?\n from the end of the line before calling
filter?
Why?
https:/