Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Convert telemetry plugin into standalone go module #3551

Closed
wants to merge 7 commits into from
Closed

Convert telemetry plugin into standalone go module #3551

wants to merge 7 commits into from

Conversation

soup-of-the-day
Copy link
Contributor

What this PR does / why we need it

Converting the telemetry plugin into a standalone go module as part of the effort to refactor tanzu-framework.

Describe testing done for PR

  • Ran existing telemetry plugin tests and rebuilt all plugins, ensured everything compiled and passed.

@soup-of-the-day soup-of-the-day requested a review from a team as a code owner October 4, 2022 21:01
Copy link
Contributor

@marckhouzam marckhouzam left a comment

Choose a reason for hiding this comment

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

Thanks @soup-of-the-day. This looks almost ready. A couple of points:

  1. You will need to explicitly call go tests from the root Makefile now that this plugin is a separate go module. Please see Run Go tests for refactored plugins #3534.
  2. In main.go, the buildinfo import should change from github.com/vmware-tanzu/tanzu-framework/pkg/v1/buildinfo to the new plugin one github.com/vmware-tanzu/tanzu-framework/cli/runtime/buildinfo. This will allow you to remove the last dependency on tanzu-framework

Copy link
Contributor

@marckhouzam marckhouzam left a comment

Choose a reason for hiding this comment

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

Thanks @soup-of-the-day.
LGTM

@codecov
Copy link

codecov bot commented Oct 5, 2022

Codecov Report

Merging #3551 (6009d6d) into main (e916c4f) will increase coverage by 0.05%.
The diff coverage is n/a.

❗ Current head 6009d6d differs from pull request most recent head 26d3587. Consider uploading reports for the commit 26d3587 to get more accurate results

@@            Coverage Diff             @@
##             main    #3551      +/-   ##
==========================================
+ Coverage   46.26%   46.32%   +0.05%     
==========================================
  Files         400      448      +48     
  Lines       39624    43871    +4247     
==========================================
+ Hits        18331    20322    +1991     
- Misses      19605    21770    +2165     
- Partials     1688     1779      +91     
Impacted Files Coverage Δ
cli/core/pkg/config/config.go 0.00% <0.00%> (-100.00%) ⬇️
cli/core/pkg/artifact/interface.go 0.00% <0.00%> (-100.00%) ⬇️
cli/core/pkg/artifact/http.go 0.00% <0.00%> (-80.49%) ⬇️
cli/core/pkg/config/bom.go 25.00% <0.00%> (-61.67%) ⬇️
cli/core/pkg/auth/csp/grpc.go 0.00% <0.00%> (-41.25%) ⬇️
cli/core/pkg/carvelhelpers/registry.go 0.00% <0.00%> (-35.42%) ⬇️
cli/core/pkg/config/defaults.go 41.50% <0.00%> (-33.97%) ⬇️
cli/core/pkg/auth/csp/token.go 64.70% <0.00%> (-18.79%) ⬇️
cli/core/pkg/auth/tkg/kube_config.go 51.87% <0.00%> (-15.04%) ⬇️
cli/core/pkg/carvelhelpers/package.go 54.28% <0.00%> (-14.29%) ⬇️
... and 96 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@marckhouzam marckhouzam left a comment

Choose a reason for hiding this comment

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

LGTM

@anujc25
Copy link
Contributor

anujc25 commented Oct 6, 2022

Considering we have bumped the go version to 1.18, you might need to run make modules again to fix the go.mod files.

Looks like PR is failing in that validation.

@soup-of-the-day
Copy link
Contributor Author

Ran make modules, thanks, @anujc25 ! Though, it resulted in an update to the go.mod of tanzu framework itself instead of the telemetry plugin.

@@ -0,0 +1,67 @@
module github.com/vmware-tanzu/tanzu-framework/cmd/cli/plugin/telemetry

go 1.18
Copy link
Contributor

Choose a reason for hiding this comment

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

Apologies for the confusion. But considering go 1.18 bump PR got reverted. We might need to update this back to 1.17

@soup-of-the-day
Copy link
Contributor Author

soup-of-the-day commented Oct 18, 2022

Back from PTO - It looks like the go 1.18 is back on main @anujc25 , so I upgraded again and rebased.

@soup-of-the-day
Copy link
Contributor Author

PR has been in limbo for a while, closing it for now.

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants