-
Notifications
You must be signed in to change notification settings - Fork 15
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
Add gitlint support to samba-container #124
Add gitlint support to samba-container #124
Conversation
The rule `check-gitlint` can be run in order to see if commits on the current branch meet the basic requirements for the project. Signed-off-by: John Mulligan <jmulligan@redhat.com>
The check-commits job will help validate the commit messages on the commits included in a PR. Signed-off-by: John Mulligan <jmulligan@redhat.com>
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.
All good. See below for a minor request.
This is similar, and inspired by, but not the same as, the install-tools.sh in samba-operator repo. Signed-off-by: John Mulligan <jmulligan@redhat.com>
I don't really like this but it matches the behavior of samba-operator while at the same time uses pattern matching so that future tools do not need a lot of boilerplate in the makefile for each instance. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
ce38e55
to
a604d36
Compare
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.
lgtm.
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.
lgtm, thanks for doing this!
Fixes: #121
I'm perfectly happy with the first commit in this series.
Everything including and after adding the 'install-tools.sh' I'm a bit iffy on. I tried multiple approaches and nothing pleased me. This current version is based on the workflow we have in samba-container where if the makefile determines a prerequisite tool is not present it tries to locally install one in the
.bin
subdir under the repo root. Unlike the samba-operator version I tried really hard to avoid adding a lot of boilerplate to the makefie now and in the future and used some pattern matching to avoid it. Still, I'm not exactly thrilled with the implementation, but it was working OK for me.