-
Notifications
You must be signed in to change notification settings - Fork 20
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
consume csgrep changes to support a new option: limit-msg-len (OSH-67) #114
consume csgrep changes to support a new option: limit-msg-len (OSH-67) #114
Conversation
@lbossis Thanks! I have not tried it myself but the code change looks good. Some minor remarks:
Implementation details can go to commit description in needed.
--- a/make-srpm.sh
+++ b/make-srpm.sh
@@ -175,6 +175,7 @@ This package contains the divine plug-in for csmock.
%package -n csmock-plugin-gitleaks
Summary: experimental csmock plug-in
Requires: csmock-common
+Requires: csdiff > 3.0.3
%description -n csmock-plugin-gitleaks
This package contains the gitleaks plug-in for csmock. Without the dependency, the scans would fail on the missing |
Removed prefix gitleaks- from FILTER_CMD which calls csgrep. Looks like it works! I executed the following command |
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.
@lbossis Thanks for the update! Now you need to squash all the fix-up commits and rewrite the commit message as described in #114 (comment). This way we end up with a clean git history where each snapshot can be built and tested. This is crucial for using git bisect later on.
@@ -124,7 +124,7 @@ Tool for plugging static analyzers into the build process, free of mock. | |||
|
|||
%package -n csmock-common | |||
Summary: Core of csmock (a mock wrapper for Static Analysis tools) | |||
Requires: csdiff > 3.0.2 | |||
Requires: csdiff > 3.0.3 |
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.
This is in fact a dependency of csmock-plugin-gitleaks
but bumping the csmock-common
requirement should be harmless, too.
25693d6
to
0579922
Compare
Hi Lukas,
Currently this piece of code looks like:
GITLEAKS_LOG = "/builddir/gitleaks-capture.log"
FILTER_CMD = "csgrep '%s' --mode=json --warning-rate-limit=%i
--limit-msg-len=%i > '%s'"
class PluginProps:
...
Do we need *two empty lines* between FILTER_CMD and class PluginsProps?
Thanks, Leo
…On Tue, Aug 15, 2023 at 12:55 PM Lukáš Zaoral ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In py/plugins/gitleaks.py
<#114 (comment)>:
> @@ -34,8 +34,7 @@
GITLEAKS_LOG = "/builddir/gitleaks-capture.log"
-FILTER_CMD = "csgrep '%s' --mode=json --warning-rate-limit=%i > '%s'"
-
Please, don't delete this empty line.
—
Reply to this email directly, view it on GitHub
<#114 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ASCNA7MLNK7CLZETWBDYIH3XVOSY3ANCNFSM6AAAAAA2ZNAYYE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Thanks for the question, @lbossis! Yes, we need two empty lines to stay conformant to the official Python Style Guide (PEP8). The section about blank lines explicitly says:
which is exactly our case! edit: typos |
45aea4b
to
d099df6
Compare
@lbossis Thanks for the update! Could you please update the commit message summary as requested at #114 (comment) ? The |
d099df6
to
94e2152
Compare
@lbossis Thanks for the update! Unfortunately the commit message needs to be reformatted like this:
The git commit summary is the first line of the commit message, delimited by a blank line from the description. The commit message summary has a special semantics in git. See the
You can easily update the commit message with: |
94e2152
to
e20c8d0
Compare
Commit message has been updated according to the recommended format.
…On Thu, Aug 17, 2023 at 7:51 AM Kamil Dudka ***@***.***> wrote:
@lbossis <https://github.com/lbossis> Thanks for the update!
Unfortunately the commit message needs to be reformatted like this:
gitleaks: implement --gitleaks-limit-msg-len defaulting to 512
This package contains an extension of gitleaks plug-in to support a new
command line option for controlling event message length from csmock.
csmock-plugin-gitleaks is dependent on the updated version of csdiff
Fixes: csutils/csdiff#114
Related: https://issues.redhat.com/browse/OSH-67
The git commit summary is the first line of the commit message, delimited
by a blank line from the description. The commit message summary has a
special semantics in git. See the git-commit(1) man page:
Though not required, it’s a good idea to begin the commit message with a single short (less than 50 character) line summarizing the change, followed by a blank line and then a more thorough description. The text up to the first blank line in a commit message is treated as the commit title, and that title is used throughout Git. For example, git-format-patch(1) turns a commit into email, and it uses the title on the Subject line and the rest of the commit in the body.
You can easily update the commit message with: git commit --amend && git
push --force
—
Reply to this email directly, view it on GitHub
<#114 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ASCNA7KIBSRAOD5CZPGAPQLXVYAS3ANCNFSM6AAAAAA2ZNAYYE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
This package contains an extension of gitleaks plug-in to support a new command line option for controlling event message length from csmock. csmock-plugin-gitleaks is dependent on the updated version of csdiff Closes: csutils#114 Related: https://issues.redhat.com/browse/OSH-67
e20c8d0
to
ac7541f
Compare
The original commit message referenced an unrelated csdiff issue by mistake (csutils/csdiff#114) so I have fixed it to make it reference this pull request instead. Merging... |
First cut