Skip to content

fix: consider all 'devel' builds as 'dev' builds #9794

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
fix: all 'devel' builds should be considered 'dev' builds.
If CI needs to be distinguished from a dev build, we should add
a different pre-release tag for those builds.
  • Loading branch information
Emyrk committed Sep 20, 2023
commit 94f621284a8b0ce7be967c278a06bcd5508ae68d
22 changes: 17 additions & 5 deletions buildinfo/buildinfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,14 @@ var (
)

const (
// develPrefix is prefixed to developer versions of the application.
develPrefix = "v0.0.0-devel"
// noVersion is the reported version when the version cannot be determined.
// Usually because `go build` is run instead of `make build`.
noVersion = "v0.0.0"

// develPreRelease is the pre-release tag for developer versions of the application.
// The pre-release tag should be appended to the version with a "-".
// Example: v0.0.0-devel
develPreRelease = "devel"
)

// Version returns the semantic version of the build.
Expand All @@ -45,7 +51,8 @@ func Version() string {
if tag == "" {
// This occurs when the tag hasn't been injected,
// like when using "go run".
version = develPrefix + revision
// <version>-<pre-release>+<revision>
version = fmt.Sprintf("%s-%s%s", noVersion, develPreRelease, revision)
return
}
version = "v" + tag
Expand All @@ -65,16 +72,21 @@ func Version() string {
func VersionsMatch(v1, v2 string) bool {
// Developer versions are disregarded...hopefully they know what they are
// doing.
if strings.HasPrefix(v1, develPrefix) || strings.HasPrefix(v2, develPrefix) {
if IsDevVersion(v1) || IsDevVersion(v2) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this logic made sense for v0.0.0, but for non-v0.0.0 we know approximately the version in question and we could require the minor version match. Or is there a use-case for returning a match for e.g. v1.1.1-devel and v1.2.3 as tested below?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking along the same lines. I will change it so only v0.0.0 matches all, and other versions do check major minor as expected.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be pretty much the previous behavior.

return true
}

return semver.MajorMinor(v1) == semver.MajorMinor(v2)
}

func IsDevVersion(v string) bool {
return strings.Contains(v, "-"+develPreRelease)
}

// IsDev returns true if this is a development build.
// CI builds are also considered development builds.
func IsDev() bool {
return strings.HasPrefix(Version(), develPrefix)
return IsDevVersion(Version())
}

// IsSlim returns true if this is a slim build.
Expand Down
6 changes: 3 additions & 3 deletions buildinfo/buildinfo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,12 @@ func TestBuildInfo(t *testing.T) {
expectMatch: true,
},
// Our CI instance uses a "-devel" prerelease
// flag. This is not the same as a developer WIP build.
// flag. This is considered the same as a developer WIP build.
{
name: "DevelPreleaseNotIgnored",
name: "DevelPreleaseIgnored",
v1: "v1.1.1-devel+123abac",
v2: "v1.2.3",
expectMatch: false,
expectMatch: true,
},
{
name: "MajorMismatch",
Expand Down