Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove deprecated cassandra flags #2789

Merged
merged 3 commits into from
Feb 5, 2021

Conversation

albertteoh
Copy link
Contributor

Signed-off-by: albertteoh albert.teoh@logz.io

Which problem is this PR solving?

Short description of the changes

  • Removes deprecated cassandra flags

Signed-off-by: albertteoh <albert.teoh@logz.io>
@albertteoh albertteoh requested a review from a team as a code owner February 5, 2021 03:11
@albertteoh albertteoh requested a review from vprithvi February 5, 2021 03:11
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.

please add changelog entry

@@ -44,7 +44,6 @@ type Configuration struct {
Port int `yaml:"port" mapstructure:"port"`
Authenticator Authenticator `yaml:"authenticator" mapstructure:",squash"`
DisableAutoDiscovery bool `yaml:"disable_auto_discovery" mapstructure:"-"`
EnableDependenciesV2 bool `yaml:"enable_dependencies_v2" mapstructure:"-"`
Copy link
Member

Choose a reason for hiding this comment

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

I expect there to be some code that actually reads this field, not seeing anything like that removed in the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did a recursive case-insensitive search in my jaeger root project dir and couldn't find EnableDependenciesV2 or enable_dependencies_v2.

Maybe something wrong with my search, can you see any references?

Copy link
Member

Choose a reason for hiding this comment

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

found it - the logic for that flag was already removed in #1364

@codecov
Copy link

codecov bot commented Feb 5, 2021

Codecov Report

Merging #2789 (628ac18) into master (3efcae5) will decrease coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2789      +/-   ##
==========================================
- Coverage   95.88%   95.83%   -0.05%     
==========================================
  Files         218      218              
  Lines        9626     9615      -11     
==========================================
- Hits         9230     9215      -15     
- Misses        327      330       +3     
- Partials       69       70       +1     
Impacted Files Coverage Δ
plugin/storage/cassandra/options.go 100.00% <ø> (ø)
cmd/query/app/static_handler.go 95.16% <0.00%> (-1.62%) ⬇️
...lugin/sampling/strategystore/adaptive/processor.go 99.07% <0.00%> (-0.93%) ⬇️

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 3efcae5...628ac18. Read the comment docs.

albertteoh added 2 commits February 5, 2021 14:26
Signed-off-by: albertteoh <albert.teoh@logz.io>
Signed-off-by: albertteoh <albert.teoh@logz.io>
@yurishkuro yurishkuro merged commit fc888d8 into jaegertracing:master Feb 5, 2021
@yurishkuro yurishkuro mentioned this pull request Feb 5, 2021
7 tasks
bhiravabhatla pushed a commit to bhiravabhatla/jaeger that referenced this pull request Feb 5, 2021
* Remove deprecated cassandra flags

Signed-off-by: albertteoh <albert.teoh@logz.io>

* Add breaking changelog entry

Signed-off-by: albertteoh <albert.teoh@logz.io>

* Fix wording; doesn't technically replace

Signed-off-by: albertteoh <albert.teoh@logz.io>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants