Coverage remained the same at 100.0% when pulling dd9ea97 on pavolloffay:fix-sed into 413942a on uber:master.

I'm getting this:

sed -i 's|"zipkincore"|"github.com/uber/jaeger/thrift-gen/zipkincore"|g' thrift-gen/agent/*.go
sed: 1: "thrift-gen/agent/agent.go": undefined label 'hrift-gen/agent/agent.go'

hmm It worked well with my sed, also with @jpkrohling and @objectiser.

sed (GNU sed) 4.2.2
Copyright (C) 2012 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by Jay Fenlason, Tom Lord, Ken Pizzini,
and Paolo Bonzini.
GNU sed home page: <http://www.gnu.org/software/sed/>.
General help using GNU software: <http://www.gnu.org/gethelp/>.
E-mail bug reports to: <bug-sed@gnu.org>.
Be sure to include the word ``sed'' somewhere in the ``Subject:'' field.

I'm using the default MacOS sed, which has no version, but its man page says

     -i extension
             Edit files in-place, saving backups with the specified extension.  If a zero-length extension is given, no
             backup will be saved.  It is not recommended to give a zero-length extension when in-place editing files, as
             you risk corruption or partial content in situations where disk space is exhausted, etc.

...

STANDARDS
     The sed utility is expected to be a superset of the IEEE Std 1003.2 (``POSIX.2'') specification.

     The -E, -a and -i options are non-standard FreeBSD extensions and may not be available on other operating systems.

HISTORY
     A sed command, written by L. E. McMahon, appeared in Version 7 AT&T UNIX.

AUTHORS
     Diomidis D. Spinellis <dds@FreeBSD.org>

BSD                              May 10, 2005                              BSD

basically the -i switch requires an argument

The -E, -a and -i options are non-standard FreeBSD extensions and may not be available on other operating systems.

I have added a simple hack. It would be better to use just the standard GNU version https://stackoverflow.com/a/34815955/4158442.

When I run it, it produces some changes like:

diff --git a/thrift-gen/jaeger/tchan-jaeger.go b/thrift-gen/jaeger/tchan-jaeger.go
index 393b8f1..89e1e9c 100644
--- a/thrift-gen/jaeger/tchan-jaeger.go
+++ b/thrift-gen/jaeger/tchan-jaeger.go
@@ -43,6 +43,10 @@ func (c *tchanCollectorClient) SubmitBatches(ctx thrift.Context, batches []*Batc
        }
        success, err := c.client.Call(ctx, c.thriftService, "submitBatches", &args, &resp)
        if err == nil && !success {
+               switch {
+               default:
+                       err = fmt.Errorf("received no result or unknown exception for submitBatches")
+               }
        }
 
        return resp.GetSuccess(), err

Is it fine?

Coverage remained the same at 100.0% when pulling 22dfcb7 on pavolloffay:fix-sed into 413942a on uber:master.

don't think the hack quite worked.

when I ran make thrift, it did not produce any differences in the existing files, but created three new files

	thrift-gen/agent/agent.go-e
	thrift-gen/agent/constants.go-e
	thrift-gen/agent/ttypes.go-e

I think the -e was interpreted as a suffix.

This is bad I will revert to pure GNU sed. Can we just use the standard GNU sed which is available on all systems? It's definitely a good thing for the community. Gnu sed can be easily installed on MacOS.

I have tried the last workaroud using -i.bak and then remove those files. But this does not prevent that new BSD commands will be added and it will fail for other contributors.

Coverage remained the same at 100.0% when pulling 93cdd4d on pavolloffay:fix-sed into 413942a on uber:master.

This is bad I will revert to pure GNU sed. Can we just use the standard GNU sed which is available on all systems? It's definitely a good thing for the community

I have mixed feelings about it, installing GNU sed is simple using brew, but one needs to finagle the path to make it the default sed that is used for everything. (Also, it would probably break other software that assumes that BSD sed is installed)

Is it too bad to have a conditional on uname -s, with the appropriate sed commands for a given environment?

@vprithvi now it should be compatible with BSD and GNU could you please test it?

Works for me, could you add a comment on why we are doing these shenanigans with sed in the makefile?

Coverage remained the same at 100.0% when pulling 69508e8 on pavolloffay:fix-sed into 413942a on uber:master.

works for me. I restarted the builds.