Add a new survey command to show Skaffold User Survey form url. by tejal29 · Pull Request #3733 · GoogleContainerTools/skaffold

tejal29 · 2020-02-26T08:24:02Z

Relates to #3011

Should merge after : #3732
Description

User facing changes

Yes. A new command added.

make
./out/skaffold survey

Thank you for offering your feedback on skaffold. Understanding your experiences and opinions helps us make skaffold better for you and other users
    Please take survey at https://forms.gle/BMTbGQXLWSdn7vEs6

To permanently disable the survey prompt, run:
   skaffold config set --survey --global disable-prompt true

Error Case testing:

Force an error in code so that actionable error message is shown

tejaldesai@tejaldesai-macbookpro2 skaffold (add_survey_command)./out/skaffold survey
Thank you for offering your feedback on Skaffold! Understanding your experiences and opinions helps us make Skaffold better for you and other users.
   Our survey can be found here: https://forms.gle/BMTbGQXLWSdn7vEs6

To permanently disable the survey prompt, run:
   skaffold config set --survey --global disable-prompt true
FATA[0000] could not update the `last-taken` field in survey config. Please run `skaffold config set --survey --global last-taken 2020-04-27T20:09:35-07:00` 
tejaldesai@tejaldesai-macbookpro2 skaffold (add_survey_command)cat ~/.skaffold/config

Test the command shown is working and updates the config.

tejaldesai@tejaldesai-macbookpro2 skaffold (add_survey_command)skaffold config set --survey --global last-taken 2020-04-27T20:09:35-07:00
set global value last-taken to 2020-04-27T20:09:35-07:00
tejaldesai@tejaldesai-macbookpro2 skaffold (add_survey_command)cat ~/.skaffold/config
global:
  survey:
    last-taken: "2020-04-27T20:09:35-07:00"
kubeContexts: []
tejaldesai@tejaldesai-macbookpro2 skaffold (add_survey_command)

Next PRs.

Wire up ShouldDisplayPrompt to show the survey prompt on every skaffold run.

The survey prompt is here

Help improve Skaffold! Take a 10 seconds anonymous survey by running
   $skaffold survey

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

Includes unit tests
Mentions any output changes.
Adds documentation as needed: user docs, YAML reference, CLI reference.
Adds integration tests if needed.

Reviewer Notes

The code flow looks good.
Unit test added.
User facing changes look good.

Release Notes

Add survey command to show Skaffold User survey url

codecov · 2020-02-26T08:33:29Z

Codecov Report

Merging #3733 into master will decrease coverage by 0.06%.
The diff coverage is 49.01%.

Impacted Files	Coverage Δ
pkg/skaffold/survey/survey.go	`31.25% <15.38%> (-68.75%)`	⬇️
cmd/skaffold/app/cmd/survey.go	`57.14% <57.14%> (ø)`
pkg/skaffold/config/util.go	`69.50% <58.62%> (-2.82%)`	⬇️
cmd/skaffold/app/cmd/cmd.go	`67.42% <100.00%> (+0.24%)`	⬆️
cmd/skaffold/app/cmd/config/set.go	`88.63% <100.00%> (+4.96%)`	⬆️
pkg/skaffold/docker/client.go	`72.27% <0.00%> (-0.28%)`	⬇️
cmd/skaffold/app/cmd/flags.go	`100.00% <0.00%> (ø)`
pkg/skaffold/config/options.go	`100.00% <0.00%> (ø)`
pkg/skaffold/schema/profiles.go	`90.96% <0.00%> (+0.05%)`	⬆️
... and 2 more

dgageot

LGTM with nits

cmd/skaffold/app/cmd/survey.go

pkg/skaffold/survey/survey.go

dgageot

Sorry, I've tested it and added more feedback

docs/content/en/docs/references/cli/_index.md

pkg/skaffold/survey/survey.go

nkubala

small nits but otherwise LGTM

pkg/skaffold/survey/survey.go

docs/content/en/docs/references/cli/_index.md

briandealwis · 2020-03-17T15:11:24Z

pkg/skaffold/survey/survey.go

+To permanently disable the survey prompt, run:
+   skaffold config set --survey --global disable-prompt true`


This should also be in the survey help text.

unfortunately, there is not method to add help in the BuilderInterface. This could be a big change since we might also need to make a change in hack/man/

nkubala · 2020-04-06T18:20:15Z

@tejal29 what's the status on this? can you check the lint errors? I'd love to get this merged

Co-Authored-By: Nick Kubala <nkubala@users.noreply.github.com>

Co-Authored-By: Brian de Alwis <bsd@acm.org>

googlebot · 2020-04-27T22:20:59Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

googlebot · 2020-04-28T03:15:14Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

googlebot · 2020-04-28T07:02:40Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

nkubala

I think we should definitely try to automatically open the survey link in a browser if we can. it would make the command feel a lot more seamless, and would probably result in more people taking the survey since they wouldn't have to manually copy-paste the link from a terminal.

cmd/skaffold/app/cmd/survey.go

docs/content/en/docs/references/cli/_index.md

pkg/skaffold/config/util.go

pkg/skaffold/survey/survey.go

nkubala · 2020-04-28T17:17:17Z

pkg/skaffold/survey/survey.go

+To permanently disable the survey prompt, run:
+   skaffold config set --survey --global disable-prompt true`


googlebot · 2020-04-28T18:02:54Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

googlebot · 2020-04-28T18:07:37Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

googlebot · 2020-04-28T18:15:19Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

Add nick's suggestion Co-Authored-By: Nick Kubala <nkubala@users.noreply.github.com>

googlebot · 2020-04-28T18:16:40Z

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

nkubala

nice, just tried this out and it opens just fine in chrome. it does still show the link to the survey in my terminal even though it just opened in my browser, but I guess that's probably fine in case it fails to open.

addressed comments.

tejal29 requested review from balopat, dgageot and nkubala as code owners February 26, 2020 08:24

pull-request-size bot added the size/M label Feb 26, 2020

googlebot added the cla: yes label Feb 26, 2020

tejal29 mentioned this pull request Feb 26, 2020

add flag --survey to set to set/unset disable survey prompt #3732

Merged

tejal29 changed the title ~~new survey command~~ Add a new survey command to show Skaffold User Survey form url. Feb 26, 2020

dgageot approved these changes Mar 4, 2020

View reviewed changes

cmd/skaffold/app/cmd/survey.go Outdated Show resolved Hide resolved

pkg/skaffold/survey/survey.go Outdated Show resolved Hide resolved

dgageot self-assigned this Mar 4, 2020

dgageot reviewed Mar 4, 2020

View reviewed changes

pkg/skaffold/survey/survey.go Outdated Show resolved Hide resolved

dgageot reviewed Mar 4, 2020

View reviewed changes

pkg/skaffold/survey/survey.go Show resolved Hide resolved

dgageot suggested changes Mar 4, 2020

View reviewed changes

dgageot previously requested changes Mar 6, 2020

View reviewed changes

docs/content/en/docs/references/cli/_index.md Outdated Show resolved Hide resolved

pkg/skaffold/survey/survey.go Outdated Show resolved Hide resolved

nkubala suggested changes Mar 10, 2020

View reviewed changes

pkg/skaffold/survey/survey.go Outdated Show resolved Hide resolved

pkg/skaffold/survey/survey.go Outdated Show resolved Hide resolved

tejal29 force-pushed the add_survey_command branch from 1a62000 to 48be044 Compare March 12, 2020 22:18

briandealwis reviewed Mar 17, 2020

View reviewed changes

tejal29 closed this Mar 23, 2020

tejal29 reopened this Mar 23, 2020

tejal29 and others added 7 commits April 27, 2020 14:56

new survey command

845d561

david's comment

c25085d

regenerate docs

e0da170

Update pkg/skaffold/survey/survey.go

c000662

Co-Authored-By: Nick Kubala <nkubala@users.noreply.github.com>

Update pkg/skaffold/survey/survey.go

280cd77

Co-Authored-By: Nick Kubala <nkubala@users.noreply.github.com>

Update pkg/skaffold/survey/survey.go

e93e391

Co-Authored-By: Brian de Alwis <bsd@acm.org>

fix lint

d8022e3

tejal29 force-pushed the add_survey_command branch from 5e3bc7f to d8022e3 Compare April 27, 2020 22:20

googlebot removed the cla: yes label Apr 27, 2020

googlebot added cla: no and removed cla: yes labels Apr 28, 2020

regenrate docs

5c339ca

tejal29 added cla: yes and removed cla: no labels Apr 28, 2020

nkubala suggested changes Apr 28, 2020

View reviewed changes

googlebot added cla: no and removed cla: yes labels Apr 28, 2020

tejal29 force-pushed the add_survey_command branch from 240332c to 4baeaab Compare April 28, 2020 18:06

tejal29 added cla: yes and removed cla: no labels Apr 28, 2020

open url

e2c9d83

tejal29 force-pushed the add_survey_command branch from 4baeaab to e2c9d83 Compare April 28, 2020 18:15

googlebot added cla: no and removed cla: yes labels Apr 28, 2020

Apply suggestions from code review

6dfea44

Add nick's suggestion Co-Authored-By: Nick Kubala <nkubala@users.noreply.github.com>

tejal29 added cla: yes and removed cla: no labels Apr 28, 2020

nkubala approved these changes Apr 28, 2020

View reviewed changes

tejal29 mentioned this pull request Apr 29, 2020

Wire up ShouldDisplayPrompt to show the survey prompt on every skaffold run. #4046

Closed

tejal29 merged commit b8fb237 into GoogleContainerTools:master Apr 29, 2020

fmotrifork mentioned this pull request May 6, 2020

skaffold 1.9.0 fishworks/fish-food#713

Merged

tejal29 deleted the add_survey_command branch April 15, 2021 07:35

		To permanently disable the survey prompt, run:
		skaffold config set --survey --global disable-prompt true`

Conversation

tejal29 commented Feb 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Feb 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dgageot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dgageot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nkubala left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

briandealwis Mar 17, 2020

Choose a reason for hiding this comment

Uh oh!

nkubala Apr 28, 2020

Choose a reason for hiding this comment

Uh oh!

tejal29 Apr 28, 2020

Choose a reason for hiding this comment

Uh oh!

nkubala commented Apr 6, 2020

Uh oh!

googlebot commented Apr 27, 2020

Uh oh!

googlebot commented Apr 28, 2020

Uh oh!

googlebot commented Apr 28, 2020

Uh oh!

nkubala left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nkubala Apr 28, 2020

Choose a reason for hiding this comment

Uh oh!

googlebot commented Apr 28, 2020

Uh oh!

googlebot commented Apr 28, 2020

Uh oh!

googlebot commented Apr 28, 2020

Uh oh!

googlebot commented Apr 28, 2020

Uh oh!

nkubala left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tejal29 commented Feb 26, 2020 •

edited

Loading

codecov bot commented Feb 26, 2020 •

edited

Loading