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

Set an empty value when a default env var value is missing #1777

Merged
merged 2 commits into from
Sep 7, 2019
Merged

Set an empty value when a default env var value is missing #1777

merged 2 commits into from
Sep 7, 2019

Conversation

jpkrohling
Copy link
Contributor

Signed-off-by: Juraci Paixão Kröhling juraci@kroehling.de

Which problem is this PR solving?

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
@objectiser
Copy link
Contributor

@jpkrohling If there is no default specified, and the env var is not set, then I would expect the behaviour to be no tag created. If the user wants to still create the tag if no env var, then that is when they would specify just the colon?

@jpkrohling
Copy link
Contributor Author

If the user wants to still create the tag if no env var, then that is when they would specify just the colon?

That's a good argument.

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
@codecov
Copy link

codecov bot commented Sep 6, 2019

Codecov Report

Merging #1777 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1777      +/-   ##
==========================================
+ Coverage   98.27%   98.27%   +<.01%     
==========================================
  Files         195      195              
  Lines        9540     9546       +6     
==========================================
+ Hits         9375     9381       +6     
  Misses        131      131              
  Partials       34       34
Impacted Files Coverage Δ
cmd/agent/app/reporter/flags.go 100% <100%> (ø) ⬆️

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 fb55050...ca28350. Read the comment docs.

Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

lgtm

@yurishkuro yurishkuro merged commit ff3d3c9 into jaegertracing:master Sep 7, 2019
bookmoons pushed a commit to bookmoons/jaeger that referenced this pull request Sep 10, 2019
…tags pair (jaegertracing#1777)

* Set an empty value when a default env var value is missing

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>

* Skip value when default isn't specified

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
@yurishkuro yurishkuro added this to the Release 1.15 milestone Nov 7, 2019
@jpkrohling jpkrohling deleted the 1776-Set-empty-when-missing-default branch July 28, 2021 19:21
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.

Panic when jaeger-tags value is set to env var without default value
3 participants