```suggestion if trimmed := bytes.TrimSpace(data); len(trimmed) == 0 { // at the moment it seems that there are "\n" before EOF, in such case we do not need to process it // this is a temporary solution to make the parser work correctly. return len(data), trimmed, nil } return len(data), data, nil ``` (if no conclusion, some comments in my mind)

```suggestion if err != nil { _ = stdoutWriter.CloseWithError(ConcatenateError(err, stderr.String())) } else { _ = stdoutWriter.Close() } ``` `stdoutWriter` shouldn't be closed twice

Why `bytes.TrimSpace` here? Above `return advance, token, nil` doesn't do that.

maybe can make `refDelim` a byte slice `[]byte`, then there is no need to do the type-cast (which needs memory allocation)

```suggestion payload := fmt.Sprintf("object %s\ntype commit\ntag %s\ntagger %s\n\n%s\n", tag.Object, tag.Name, ref["creator"], strings.TrimSpace(tag.Message)) ```

TestParser maybe a better name.

Please place a blank line between internal packages and external packages.

Maybe we should not delete it but use an insert-update model?

If the `stdout` maybe very large, we can use `RunWithContext` and use a `io.Pipe` to create a reader for `Parser` and a writer for `Git`.

As above

Please add copyright head.

note: this is the main improvement.

note: unsure of (1) what the payload should look like, (2) if it's even needed in this context. To me it _seemed_ like it was pulled out as part of extracting commit metadata and ended up in the tag also "as a bonus". It _seems_ to me like the payload _could_ be synthesized from the fields, but unsure if it's needed. Would appreciate guidance.

note: a bit unsure about need to sort here, guidance appreciated.

note: if the overall approach seems reasonable I'll add unit tests for the `Parser` type.

note: if the overall approach seems reasonable I'll add unit tests for the `Format` type.