Skip to content

Read release info from build info#652

Closed
mattrobenolt wants to merge 1 commit intogetsentry:masterfrom
mattrobenolt:buildinfo
Closed

Read release info from build info#652
mattrobenolt wants to merge 1 commit intogetsentry:masterfrom
mattrobenolt:buildinfo

Conversation

@mattrobenolt
Copy link
Copy Markdown
Contributor

Since go1.18, vcs.revision gets compiled into the binary and can be read.

Conveniently, debug.ReadBuildInfo has been around since go1.12, so there's no need to conditionally compile this unless sentry-go needs to support < go1.12 for some reason.

So in builds newer than 1.18, this key just simply won't exist and effectively will do nothing.

This is much more convenient to detect in anything go1.18 since it's much more common to have this compiled in.

Since go1.18, vcs.revision gets compiled into the binary and can be
read.

Conveniently, debug.ReadBuildInfo has been around since go1.12, so
there's no need to conditionally compile this unless sentry-go needs to
support < go1.12 for some reason.

So in builds newer than 1.18, this key just simply won't exist and
effectively will do nothing.

This is much more convenient to detect in anything go1.18 since it's
much more common to have this compiled in.
@cleptric
Copy link
Copy Markdown
Member

The SDK requires baseline Go 1.18, so this is no issue

go 1.18

Any chance you could add some tests?

@mattrobenolt
Copy link
Copy Markdown
Contributor Author

Any chance you could add some tests?

No, I'm not being paid by Sentry.

@codecov
Copy link
Copy Markdown

codecov bot commented Jun 12, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.12 ⚠️

Comparison is base (97a00a4) 80.65% compared to head (dd4066e) 80.54%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #652      +/-   ##
==========================================
- Coverage   80.65%   80.54%   -0.12%     
==========================================
  Files          43       43              
  Lines        4332     4338       +6     
==========================================
  Hits         3494     3494              
- Misses        712      718       +6     
  Partials      126      126              
Impacted Files Coverage Δ
util.go 57.37% <0.00%> (-6.26%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@kamilogorek
Copy link
Copy Markdown
Contributor

Hi Matt.

@mattrobenolt
Copy link
Copy Markdown
Contributor Author

Hi Matt.

👋 just trying to use your software over here.

@cleptric
Copy link
Copy Markdown
Member

Any chance you could add some tests?

No, I'm not being paid by Sentry.

I understand, but we would like to handle this PR as any other PRs we receive from contributors.
I'll leave this open in the meantime.

@mattrobenolt
Copy link
Copy Markdown
Contributor Author

Yeah and that's fine. You're welcome to add your own tests. I'm not sure how this is easily testable and it's a trivial change. But since you're insisting, there's no tests at all for this entire function.

We've already worked around this by simply using my code in our own config. Figured I'd try to upstream it since it makes sense for others.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants