Skip to content

Commit

Permalink
addressed minor pr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
JulesFaucherre committed Aug 1, 2023
1 parent f79234b commit c357de6
Show file tree
Hide file tree
Showing 9 changed files with 26 additions and 24 deletions.
5 changes: 3 additions & 2 deletions Taskfile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,9 @@ tasks:
build:
desc: Build main
cmds:
# LDFlags sets the segment endpoint to an empty string so that all it is replaced by the default value
- go build -v -o build/darwin/amd64/circleci -ldflags="-X 'github.com/CircleCI-Public/circleci-cli/telemetry.SegmentEndpoint=' -X 'github.com/CircleCI-Public/circleci-cli/telemetry.SegmentKey=AbgkrgN4cbRhAVEwlzMkHbwvrXnxHh35'" .
# LDFlags sets the segment endpoint to an empty string thus letting the analytics library set the default endpoint on its own
# Not setting the `SegmentEndpoint` variable would let the value in the code ie "http://localhost"
- go build -v -o build/$(go env GOOS)/$(go env GOARCH)/circleci -ldflags="-X 'github.com/CircleCI-Public/circleci-cli/telemetry.SegmentEndpoint=' -X 'github.com/CircleCI-Public/circleci-cli/telemetry.SegmentKey="`echo ${SEGMENT_KEY}`"'" .

build-linux:
desc: Build main
Expand Down
2 changes: 1 addition & 1 deletion clitest/clitest.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func WithTempSettings() *TempSettings {
tempSettings.Config = OpenTmpFile(settingsPath, "cli.yml")
tempSettings.Telemetry = OpenTmpFile(settingsPath, "telemetry.yml")
content, err := yaml.Marshal(settings.TelemetrySettings{
IsActive: false,
IsEnabled: false,
HasAnsweredPrompt: true,
})
gomega.Expect(err).ToNot(gomega.HaveOccurred())
Expand Down
12 changes: 6 additions & 6 deletions cmd/create_telemetry/create_telemetry.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func CreateTelemetry(config *settings.Config) telemetry.Client {
}

loadTelemetrySettings(&telemetrySettings, &user, apiClient, ui)
client := telemetry.CreateClient(user, telemetrySettings.IsActive)
client := telemetry.CreateClient(user, telemetrySettings.IsEnabled)

return client
}
Expand All @@ -104,7 +104,7 @@ func loadTelemetrySettings(settings *settings.TelemetrySettings, user *telemetry
// If we already have telemetry information or that telemetry is explicitly disabled, skip
if settings.HasAnsweredPrompt {
// If we have no user id, we try requesting the user id again
if settings.UserID == "" && settings.IsActive {
if settings.UserID == "" && settings.IsEnabled {
myID, err := apiClient.GetMyUserId()
if err == nil {
settings.UserID = myID
Expand All @@ -120,7 +120,7 @@ func loadTelemetrySettings(settings *settings.TelemetrySettings, user *telemetry

// If stdin is not available, send telemetry event, disable telemetry and return
if !isStdinATTY {
settings.IsActive = false
settings.IsEnabled = false
err := telemetry.SendTelemetryApproval(telemetry.User{
UniqueID: settings.UniqueID,
}, telemetry.NoStdin)
Expand All @@ -136,11 +136,11 @@ func loadTelemetrySettings(settings *settings.TelemetrySettings, user *telemetry
fmt.Println("Participation is voluntary, and your choice can be changed at any time through the command `cli telemetry enable` and `cli telemetry disable`.")
fmt.Println("For more information, please see our privacy policy at https://circleci.com/legal/privacy/.")
fmt.Println("")
settings.IsActive = ui.AskUserToApproveTelemetry("Enable telemetry?")
settings.IsEnabled = ui.AskUserToApproveTelemetry("Enable telemetry?")
settings.HasAnsweredPrompt = true

// Make sure we have user info and set them
if settings.IsActive {
if settings.IsEnabled {
if settings.UniqueID == "" {
settings.UniqueID = CreateUUID()
}
Expand All @@ -159,7 +159,7 @@ func loadTelemetrySettings(settings *settings.TelemetrySettings, user *telemetry

// Send telemetry approval event
approval := telemetry.Enabled
if !settings.IsActive {
if !settings.IsEnabled {
approval = telemetry.Disabled
}

Expand Down
6 changes: 3 additions & 3 deletions cmd/create_telemetry/create_telemetry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func TestLoadTelemetrySettings(t *testing.T) {
},
want: want{
settings: settings.TelemetrySettings{
IsActive: true,
IsEnabled: true,
HasAnsweredPrompt: true,
UserID: userId,
UniqueID: uniqueId,
Expand All @@ -126,7 +126,7 @@ func TestLoadTelemetrySettings(t *testing.T) {
},
want: want{
settings: settings.TelemetrySettings{
IsActive: false,
IsEnabled: false,
HasAnsweredPrompt: true,
},
telemetryEvents: []telemetry.Event{
Expand All @@ -146,7 +146,7 @@ func TestLoadTelemetrySettings(t *testing.T) {
},
want: want{
settings: settings.TelemetrySettings{
IsActive: true,
IsEnabled: true,
HasAnsweredPrompt: true,
UserID: userId,
UniqueID: "other-id",
Expand Down
2 changes: 1 addition & 1 deletion cmd/telemetry.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func setIsTelemetryActive(apiClient create_telemetry.TelemetryAPIClient, isActiv
}

settings.HasAnsweredPrompt = true
settings.IsActive = isActive
settings.IsEnabled = isActive

if settings.UniqueID == "" {
settings.UniqueID = create_telemetry.CreateUUID()
Expand Down
12 changes: 6 additions & 6 deletions cmd/telemetry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,15 @@ func TestSetIsTelemetryActive(t *testing.T) {
apiClient: telemetryTestAPIClient{},
isActive: true,
settings: &settings.TelemetrySettings{
IsActive: false,
IsEnabled: false,
HasAnsweredPrompt: true,
UniqueID: uniqueId,
UserID: userId,
},
},
want: want{
settings: &settings.TelemetrySettings{
IsActive: true,
IsEnabled: true,
HasAnsweredPrompt: true,
UniqueID: uniqueId,
UserID: userId,
Expand All @@ -69,7 +69,7 @@ func TestSetIsTelemetryActive(t *testing.T) {
},
want: want{
settings: &settings.TelemetrySettings{
IsActive: true,
IsEnabled: true,
HasAnsweredPrompt: true,
UniqueID: uniqueId,
UserID: userId,
Expand All @@ -82,15 +82,15 @@ func TestSetIsTelemetryActive(t *testing.T) {
apiClient: telemetryTestAPIClient{},
isActive: false,
settings: &settings.TelemetrySettings{
IsActive: true,
IsEnabled: true,
HasAnsweredPrompt: true,
UniqueID: uniqueId,
UserID: userId,
},
},
want: want{
settings: &settings.TelemetrySettings{
IsActive: false,
IsEnabled: false,
HasAnsweredPrompt: true,
UniqueID: uniqueId,
UserID: userId,
Expand All @@ -106,7 +106,7 @@ func TestSetIsTelemetryActive(t *testing.T) {
},
want: want{
settings: &settings.TelemetrySettings{
IsActive: false,
IsEnabled: false,
HasAnsweredPrompt: true,
UniqueID: uniqueId,
UserID: userId,
Expand Down
1 change: 1 addition & 0 deletions cmd/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ func newVersionCommand(config *settings.Config) *cobra.Command {
opts.args = args
},
Run: func(_ *cobra.Command, _ []string) {
fmt.Printf("telemetry.SegmentKey = %+v\n", telemetry.SegmentKey)
fmt.Printf("%s+%s (%s)\n", version.Version, version.Commit, version.PackageManager())
},
}
Expand Down
2 changes: 1 addition & 1 deletion settings/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ type UpdateCheck struct {

// TelemetrySettings is used to represent telemetry related settings
type TelemetrySettings struct {
IsActive bool `yaml:"is_active"`
IsEnabled bool `yaml:"is_enabled"`
HasAnsweredPrompt bool `yaml:"has_answered_prompt"`
UniqueID string `yaml:"unique_id"`
UserID string `yaml:"user_id"`
Expand Down
8 changes: 4 additions & 4 deletions telemetry/telemetry.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ func (cli nullClient) Track(_ Event) error { return nil }
// Used when telemetry is enabled

type segmentClient struct {
cli analytics.Client
user User
analyticsClient analytics.Client
user User
}

func newSegmentClient(user User) Client {
Expand Down Expand Up @@ -145,15 +145,15 @@ func (segment *segmentClient) Track(event Event) error {
event.Properties["team_name"] = segment.user.TeamName
}

return segment.cli.Enqueue(analytics.Track{
return segment.analyticsClient.Enqueue(analytics.Track{
UserId: segment.user.UniqueID,
Event: event.Object,
Properties: event.Properties,
})
}

func (segment *segmentClient) Close() error {
return segment.cli.Close()
return segment.analyticsClient.Close()
}

// File telemetry
Expand Down

0 comments on commit c357de6

Please sign in to comment.