Minor patch to tools/check_format.py to fix #2391 .#2454
Minor patch to tools/check_format.py to fix #2391 .#2454mattklein123 merged 1 commit intoenvoyproxy:masterfrom srikailash:master
Conversation
mattklein123
left a comment
There was a problem hiding this comment.
thanks! does this fix #2391? If so can you mark the description as such? Also please fix DCO.
tools/check_format.py
Outdated
There was a problem hiding this comment.
please remove commented out code
There was a problem hiding this comment.
Hello Matt , thanks for looking at it . pushed a fix with removing commented code . Yes , it fixes #2391 , will add that to description . can you please elaborate on what fixing DCO means ?
…changing directory and using relative path . Signed-off-by: sri kailash <sri.gandebathula@booking.com>
mattklein123
left a comment
There was a problem hiding this comment.
@danielhochman @htuch any concerns here? Seems fine to me. Not sure why chdir was done in the first place (probably by me once upon a time).
htuch
left a comment
There was a problem hiding this comment.
LGTM; the only reason I can think of for the chdir is if we're using this from a consuming repository like you do at Lyft. If it works for you, ship it.
* move node_info_cache to common * add a new method to also fetch peer metadata id. * get peer node info inside root context * add capability to disabel node cache * make node pointer return const * fix test
Description:
Fixes #2391 .
Reason for Issue :
If a directory is passed to the script , current directory is changed and relative path is used .
When /tools/header.py is called with relative path it couldn't find the file .
Fix : fixed by using absolute path instead of changing directory and using relative path .
Risk Level:
Small bug fix for tooling
Testing:
Tested with this one-liner(./tools/check_format.py check source/common/stats) and seems to work fine
Docs Changes:
N/A
Release Notes:
Now directories can also be passed as an argument to /tools/check_format.py .
[Optional Fixes #Issue]