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

Add a message on "okteto down" for discoverability of "okteto push" #770

Merged
merged 1 commit into from
Mar 21, 2020

Conversation

pchico83
Copy link
Contributor

Proposed changes

  • Add a message on "okteto down" for discoverability of "okteto push"

Copy link
Member

@rberrelleza rberrelleza left a comment

Choose a reason for hiding this comment

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

I don't like this, I don't think it provides any value to the user and it's kind of annoying to see it everytime you run down.

@rlamana
Copy link
Contributor

rlamana commented Mar 20, 2020

@rberrelleza I honestly see a lot of value in providing information and helping the discoverability of other features in the tool, which is hard to achieve in a CLI tool.

As an example of other tools doing something similar, git constantly tells you what to do next in some of its commands. For example:

$ git status
On branch master
Your branch is behind 'origin/master' by 1 commit, and can be fast-forwarded.
  (use "git pull" to update your local branch)

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

	modified:   index.html

no changes added to commit (use "git add" and/or "git commit -a")

All that to help users with next steps and feature discoverability.

And that one is pretty verbose and we use it all the time. I think the one @pchico83 proposes is not that intrusive or annoying. And okteto down is not something you keep doing all the time.

@rlamana
Copy link
Contributor

rlamana commented Mar 20, 2020

@rberrelleza we could maybe create a different kind of information output that is not so prominent, instead of the intense blue we use for information. Would that help to make it less annoying?

@rberrelleza
Copy link
Member

@rlamana I like the concept of the hint, in git it works pretty well (e.g. the suggesting to create a PR after you push a change). I'm not sure that after down, push is the action that should go, but if you and @pchico83 like it, let's go for it.

cmd/down.go Outdated
@@ -53,6 +53,7 @@ func Down() *cobra.Command {
}

log.Success("Development environment deactivated")
log.Information("Execute 'okteto push' to permanently apply your changes to your cluster")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
log.Information("Execute 'okteto push' to permanently apply your changes to your cluster")
log.Information("Run 'okteto push' to deploy your local changes in your cluster")

Signed-off-by: Pablo Chico de Guzman <pchico83@gmail.com>
@codecov
Copy link

codecov bot commented Mar 21, 2020

Codecov Report

Merging #770 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #770      +/-   ##
==========================================
- Coverage   31.99%   31.98%   -0.01%     
==========================================
  Files          62       62              
  Lines        4820     4821       +1     
==========================================
  Hits         1542     1542              
- Misses       3159     3160       +1     
  Partials      119      119
Impacted Files Coverage Δ
cmd/down.go 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2344cb5...3ccd37d. Read the comment docs.

@pchico83 pchico83 merged commit e2205f4 into master Mar 21, 2020
@pchico83 pchico83 deleted the push branch March 21, 2020 07:36
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.

None yet

3 participants