-
Notifications
You must be signed in to change notification settings - Fork 440
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
internal: check for new majors in v2 #3078
base: v2-dev
Are you sure you want to change the base?
Conversation
Datadog ReportBranch report: ✅ 0 Failed, 4056 Passed, 64 Skipped, 2m 49.37s Total Time |
BenchmarksBenchmark execution time: 2025-01-13 19:12:02 Comparing candidate commit 0b4fcd0 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 51 metrics, 2 unstable metrics. |
return "", fmt.Errorf("no valid versions found") | ||
} | ||
|
||
// // walk through contrib directory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO(quinna): remove these before merge.
leaving comments for clarity for reviewers.
@@ -804,6 +804,10 @@ var packages = map[Package]PackageInfo{ | |||
}, | |||
} | |||
|
|||
func GetPackages() map[Package]PackageInfo { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a duplicate from https://github.com/DataDog/dd-trace-go/pull/3070/files#diff-361675d30208c976289abde824bc8391916548f27a769c8264f036a4daf420f1R850
so it depends on which PR gets merged first.
} | ||
|
||
func main() { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// ValidateRepository checks if the repository string starts with "github.com" and ends with a version suffix. | ||
// Returns true if the repository is valid, false otherwise. | ||
func ValidateRepository(repo string) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: as this is a CLI tool, you don't need to export functions. Usually, in these cases, you would do validateRepository
and the same for the other functions that start with uppercase.
continue | ||
} | ||
|
||
fmt.Printf("Base repo: %s\n", baseRepo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: if we can use log
instead of fmt
it would be cleaner and allow to parse a bit the output.
const prefix = "github.com/" | ||
if !strings.HasPrefix(repo, prefix) { | ||
return false | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not all packages have a github.com
prefix: cloud.google.com/go/pubsub.v1
is an example.
What does this PR do?
Checks for new major versions on github in v2
Motivation
In the original PR, we used a hardcoded, manual
integration_go.mod
to detect if new major versions (corresponding to new packages) have been released. However, inv2
each package will be treated as its own contrib with its own correspondinggo.mod
. We want to parse thesego.mod
files to check the currentlatest
major, and output if a new major is available.This will write output to
latests.txt
and output if a new major package is available.Reviewer's Checklist
v2-dev
branch and reviewed by @DataDog/apm-go.Unsure? Have a question? Request a review!