Skip to content
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

chore: bump go module path #3926

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

davidgiga1993
Copy link

@davidgiga1993 davidgiga1993 commented Jul 31, 2024

What this PR does:
Changes the module path to match the major version, this resolves go get not working.

This change was based upon the changes required for the v3 loki release

Which issue(s) this PR fixes:
Fixes #3817

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@CLAassistant
Copy link

CLAassistant commented Jul 31, 2024

CLA assistant check
All committers have signed the CLA.

@davidgiga1993 davidgiga1993 marked this pull request as draft July 31, 2024 13:28
@davidgiga1993 davidgiga1993 marked this pull request as ready for review July 31, 2024 13:36
# Conflicts:
#	integration/bench/load_test.go
#	integration/e2e/api_test.go
#	integration/e2e/backend/backend.go
#	integration/e2e/compression_test.go
#	integration/e2e/cross_cluster_reads_test.go
#	integration/e2e/e2e_test.go
#	integration/e2e/encodings_test.go
#	integration/e2e/https_test.go
#	integration/e2e/limits_test.go
#	integration/e2e/metrics_generator_test.go
#	integration/e2e/multi_tenant_test.go
#	integration/e2e/overrides_test.go
#	integration/e2e/query_plugin_test.go
#	integration/e2e/receivers_test.go
#	integration/e2e/serverless/serverless_test.go
#	integration/poller/poller_test.go
#	modules/frontend/combiner/common_test.go
#	modules/frontend/pipeline/collector_http.go
#	modules/generator/instance.go
#	modules/generator/instance_test.go
#	pkg/traceql/util.go
@davidgiga1993
Copy link
Author

I just pulled the latest changes on main into this. As a merge conflict will arise from every new change on main it would be great if someone could provide feedback for this change, thanks!

@zalegrala
Copy link
Contributor

Hi @davidgiga1993. Does this show up because you are vendoring Tempo somewhere? I remember seeing the Loki team made this change as you point out, but didn't follow the results closely to know how it went.

@davidgiga1993
Copy link
Author

@zalegrala yes we're trying to use Tempo as package for a custom reverse proxy to parse the AST of the query. We're doing the same thing with Prometheus and Loki (which can be consumed as a dependency)

@zalegrala
Copy link
Contributor

This change seems reasonable to me. We hesitate because this will cause some additional work for us internally. However, it might be nice to get this in before 2.6. If this can get updated I can help review early next week.

@davidgiga1993
Copy link
Author

This change seems reasonable to me. We hesitate because this will cause some additional work for us internally. However, it might be nice to get this in before 2.6. If this can get updated I can help review early next week.

I've resolved the conflicts

@davidgiga1993
Copy link
Author

And again

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.

Module path needs to be updated with '/v2'
3 participants