getsentry / raven-go

Sentry client in Go
https://sentry.io
BSD 3-Clause "New" or "Revised" License
561 stars 148 forks source link

breaking change in v0.1.1 #220

Closed pierrre closed 5 years ago

pierrre commented 5 years ago

Hello,

I've noticed there is a breaking change between v0.1.0 and v0.1.1. https://github.com/getsentry/raven-go/commit/84d70c6389014a3e6d6692dea9e6df3fe5cef1ec There is a new parameter in NewStacktraceFrame(). (yes, I'm using it in my code)

Is it really OK to break the API for a new "patch" version ?

dcramer commented 5 years ago

That’s a fair point. I’m not sure we’ve set clear boundaries on what is public and what is private (but accessible). I’d be fine backing out the 0.1.1 release and pushing it to 0.2.0.

pierrre commented 5 years ago

Actually, it was really easy to fix on my side. So it's not really a problem for me anymore. I was just reporting the breaking change, which was unexpected.

pierrre commented 5 years ago

About what is public and what is private (but accessible), I'm calling NewStacktraceFrame() in my code, because I use my own "error library" for stack traces.

In the beginning, I was using github.com/pkg/errors but raven-go didn't support it. So I wrote my own wrapper for raven-go that extracts the stack trace from errors.

Later, I decided to write my own "fork" of github.com/pkg/errors for various reasons:

Then raven-go started to support stack traces from github.com/pkg/errors.

The problem is that it's not compatible with my own error library, which uses this interface:

    type stackFramer interface {
        StackFrames() *runtime.Frames
    }

So, it's very important for me that you keep NewStacktraceFrame() public or accessible.

asdine commented 5 years ago

I think a v0.2.0 release would be better. Even if the SemVer spec doesn't guarantee stability for anything in the v0 branch, having a minor version breaking things instead of a patch version gives more flexibility. It allows for example to use the ~v0.1.0 range and expect further patches to not break anything.

dcramer commented 5 years ago

Will removing the 0.1.1 tag be an issue? I can also replace it with 0.1.0? Will take care of this later this afternoon, and appreciate everyone’s feedback!

pierrre commented 5 years ago

I'm already using v0.1.1, and I've fixed my code for the breaking change. It's not a huge problem for me if you remove this tag.

I don't know what is the best practice in this situation. Maybe you could release a new tag v0.1.2 that reverts the breaking change.

dcramer commented 5 years ago

0.1.2 seems like a good solution. If anyone finds concerns with that lmk.

dcramer commented 5 years ago

All set. v0.1.2 and v0.2.0 exist.

pierrre commented 5 years ago

thank you 👍

asdine commented 5 years ago

Thank you 👍