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

Improve error messages #639

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dutradda
Copy link

Add some suggestions about the origin of error

@codecov-commenter
Copy link

codecov-commenter commented Jul 14, 2020

Codecov Report

Merging #639 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #639   +/-   ##
=======================================
  Coverage   50.25%   50.25%           
=======================================
  Files          86       86           
  Lines        6230     6230           
=======================================
  Hits         3131     3131           
  Misses       2887     2887           
  Partials      212      212           

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 51ad1ae...c054273. Read the comment docs.

@@ -12,8 +12,8 @@ var (
ErrInvalidName = status.Errorf(codes.InvalidArgument, "Invalid App Name")
ErrInvalidLimits = status.Errorf(codes.InvalidArgument, "Invalid Limits")
ErrInvalidAutoscale = status.Errorf(codes.InvalidArgument, "Invalid Autoscale")
ErrInvalidEnvVarName = status.Errorf(codes.InvalidArgument, "Invalid Env Var Name")
ErrInvalidSecretName = status.Errorf(codes.InvalidArgument, "Invalid Secret Name")
ErrInvalidEnvVarName = status.Errorf(codes.InvalidArgument, "Invalid Env Var Name (Could be a secret already set)")
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe use Invalid Env Var Name, or Env Var already set instead, since it gives the impression that you can use a secret or env var that is already set (that's just 💅 though).

Also, I think you should swap the message here with the message below

Suggested change
ErrInvalidEnvVarName = status.Errorf(codes.InvalidArgument, "Invalid Env Var Name (Could be a secret already set)")
ErrInvalidEnvVarName = status.Errorf(codes.InvalidArgument, "Invalid Env Var Name (Could be a env var already set)")

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.

3 participants