fix: same secret should be shown once in the results output for scan command #430
Conversation
Codecov Report
@@ Coverage Diff @@
## main #430 +/- ##
==========================================
+ Coverage 94.51% 94.53% +0.02%
==========================================
Files 83 83
Lines 3534 3534
==========================================
+ Hits 3340 3341 +1
+ Misses 194 193 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
ggshield/scan/repo.py
Outdated
| for commit in commits: | ||
| for file in commit.files: | ||
| commit_files_tuple.append((commit, file)) |
There was a problem hiding this comment.
I would rename commit_files_tuple to commit_files_tuples, since its a list of tuples and not a list.
Also, I think you can create it using a comprehension list, like this (untested):
commit_files_tuples = [(c, f) for c in commits for f in c.files]
There was a problem hiding this comment.
indeed, thanks !
ggshield/scan/repo.py
Outdated
| for file in commit.files | ||
| ) | ||
| ] | ||
| for (commit, file), result in zip(commit_files_tuple, results.results): |
There was a problem hiding this comment.
Looks like we do not use file in this loop, so it can be replaced with _:
| for (commit, file), result in zip(commit_files_tuple, results.results): | |
| for (commit, _), result in zip(commit_files_tuple, results.results): |
It's interesting that this new code is probably going to be faster! 😀
There was a problem hiding this comment.
@agateau-gg
Side note : if I am not mistaken we do not benchmark scan repo in the performance benchmark ? We could have seen it here. But it may be too long to include the repo option in the benchmark ?
There was a problem hiding this comment.
We benchmark scan commit-range, which uses this code. But the benchmark does not show a significant change :/.
alina-tuholukova-gg
force-pushed
the
alina/cor-769/fix-double-policy-break-in-result
branch
from
7e85e09
to
c278228
Compare
agateau-gg
deleted the
alina/cor-769/fix-double-policy-break-in-result
branch
Fixes issue #428