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.

Sign up for GitHub

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

Jump to bottom

Deprecate span map cache flag #5551

Merged
merged 22 commits into from
Apr 22, 2020
Merged

Deprecate span map cache flag #5551

merged 22 commits into from
Apr 22, 2020

Conversation

shayzluf
Copy link
Contributor

@shayzluf shayzluf commented Apr 21, 2020 •
edited by rauljordan
Loading

Resolves #5550

Description

deprecate span map cache flag

@shayzluf shayzluf requested a review from a team as a code owner April 21, 2020 09:55
nisdas
nisdas requested changes Apr 21, 2020
View reviewed changes
Copy link
Member

@nisdas nisdas left a comment •
edited
Loading

Choose a reason for hiding this comment

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

So this flag was never part of the feature config, is there any reason why you need to add to it there? why not add in deprecated flags in the slasher's flag folders instead and use it from there.

slasher/node/node.go Outdated Show resolved Hide resolved
@shayzluf
Copy link
Contributor Author

So this flag was never part of the feature config, is there any reason why you need to add to it there? why not add in deprecated flags in the slasher's flag folders instead and use it from there.

thought of adding the canonical method to deprecate flags

nisdas
nisdas previously approved these changes Apr 21, 2020
View reviewed changes
rauljordan
rauljordan reviewed Apr 21, 2020
View reviewed changes
slasher/usage_test.go Outdated Show resolved Hide resolved
rauljordan
rauljordan previously approved these changes Apr 21, 2020
View reviewed changes
@codecov
Copy link

codecov bot commented Apr 22, 2020

Codecov Report

Merging #5551 into master will increase coverage by 18.21%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           master    #5551       +/-   ##
===========================================
+ Coverage    4.35%   22.56%   +18.21%     
===========================================
  Files         236      206       -30     
  Lines       20548    17520     -3028     
===========================================
+ Hits          895     3954     +3059     
+ Misses      19503    12762     -6741     
- Partials      150      804      +654

@prylabs-bulldozer prylabs-bulldozer bot merged commit 9c012cc into master Apr 22, 2020
@delete-merged-branch delete-merged-branch bot deleted the deprecate_span_cache branch April 22, 2020 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rauljordan rauljordan rauljordan approved these changes

@nisdas nisdas nisdas left review comments

@0xKiwi 0xKiwi Awaiting requested review from 0xKiwi

@terencechain terencechain Awaiting requested review from terencechain

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Deprecate span map cache flag in slasher
3 participants
@shayzluf @rauljordan @nisdas