-
Notifications
You must be signed in to change notification settings - Fork 14
Added conversion for str for git timestamps #216
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
Conversation
@filipecosta90, could you please look at change? it was required to remove variable which never used in stats.py to make tests in CI passing. If this should be fixed in another way, please let me know. |
@@ -338,9 +338,6 @@ def generate_stats_cli_command_logic(args, project_name, project_version): | |||
logging.warn("Unable to detect group in {}".format(cmd)) | |||
|
|||
priority_list = sorted(((priority[cmd], cmd) for cmd in priority), reverse=True) | |||
priority_list_usecs = sorted( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@markovamaria why was this removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@markovamaria why was this removed?
It is assigned, but never used. This line causing fail in .tox tools.
Fail in actions in here: https://github.com/redis/redis-benchmarks-specification/actions/runs/4246038262
Manual repro:
$ .tox/integration-tests/bin/flake8 redis_benchmarks_specification
redis_benchmarks_specification/__cli__/stats.py:341:9: F841 local variable 'priority_list_usecs' is assigned to but never used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@markovamaria why was this removed?
I agree that it is separate issue. :) But it blocked CI for this commit
@filipecosta90, so what do you think, may I keep removing of unused var in this PR or should do separate one? Alternatively need to use this variable somehow.. |
Added conversion to avoid error during testing triggering: