-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Why sign each and every commit? #866
Comments
Patch is created by maintainers, not by the author. Whether the maintainer squashes commits (our default) or not, in both cases we are dealing with at least one code change that is not signed, and therefore not attested to its origin by the author. |
I personally sign all my commits as a "good practice", to signal that I'm indeed the one who did the commit. For my contributions on Jaeger repositories, the commit signature is stripped out most of the time as the commits are usually squashed.
It can be done automatically by having something like this on
Not quite sure I understand what you mean here. Do you have a concrete example of what went wrong? |
Yes: jaegertracing/jaeger-client-go#274. In the client repo, began working off of a branch that was not fully signed by a coworker. Was not able to fix without squashing. See this comment: jaegertracing/jaeger-client-go#274 (comment). |
I want the config setting without GPG. |
Actually, envoy has a solution using a pre-commit hook: envoyproxy/envoy#2283. |
I guess you are talking about "signing-off" the commit ( |
Requirement - what kind of business use case are you trying to solve?
General issue in contributing.
Problem - what in Jaeger blocks you from solving the requirement?
It is difficult to sign each and every commit. One bad commit ruins the whole history. The only way to fix it is to squash or to do some esoteric git magic.
Proposal - what do you suggest to solve the problem or improve the existing situation?
Only require signing the patch resulting from the pull request.
Any open questions to address
If there is a good legal reason, we should add it to the docs.
The text was updated successfully, but these errors were encountered: