-
Notifications
You must be signed in to change notification settings - Fork 79
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
Allow signed off via comment #83
Comments
Yes please!! |
Theoretically this can be solved by the button in #79; however, the does require action by someone with write access, so it's up to maintainers to determine if the contributor has sufficiently commented that they want this. However, having the actual bot re-write the commits would not work as intended. DCO is a GitHub App, not OAuth, so it can not act as a user to re-write git history. If the DCO re-wrote commits, they would be attributed to the bot, not the original author. |
Why can't it rewrite on behalf of the user? What is the technical issue? I can make a commit look like it comes from anyone. |
Well the whole point of the DCO is to ensure valid commits by users not bots, so this feels like a bad thing for the app to do to me at least. Additionally, the DCOs current permissions are very minimal. It currently has: I can see a lot of reasons why folks wouldn't want to give a GitHub App write access to their repo contents. I'm opposed to implementing this in a way where the bot actually rewrites the commits. I'm fine with a button that says 'set status to passing' or something like that which could be pressed by maintainers or users, or adding instructions with the exact commands for users to run to fix their commits. All of this is implemented in #79. |
Here is how Chef handles that....
@caniszczyk what does the linux foundation / CNCF think of this? |
I don't understand the hair splitting on the DCO coming from the user via
the `-s` flag on git commit or a comment on GitHub which authorizes a bot
to rebase and add the `-s` flag.
If the Chef workaround is acceptable that is fine though
…On Tue, Aug 28, 2018 at 6:54 AM Matt Farina ***@***.***> wrote:
Here is how Chef handles that....
Every commit, that does not meet the criteria for an obvious fix, must
have a Signed-off-by line. Chef maintainers, listed in the project’s
MAINTAINERS.md file, may ask you to attest to the DCO by way of a comment
on your PR. After you have done so, a maintainer will rebase your branch
adding an appropriate attestation to each commit. The resulting commit
message would include something along the lines of:
Signed-off-by: Jane Doe ***@***.***> on behalf of Joe Smith ***@***.***>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#83 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AACDCNad6gSjAZU3tAt2qj4QZ9mlyZ8Fks5uVUuVgaJpZM4Vkph7>
.
|
@philips I'm most interested in the easiest solution that the lawyers will accept. One issue is that a bot changing the repo means the local users commits are different from the ones on the PR. This is going to create a different experience to force pull (will that work?) to replace your local commits with the bot altered commits on the same branch github is tracking against the PR. |
Who talked to a lawyer about the bot? Which lawyer? And what was their
response?
…On Tue, Aug 28, 2018 at 12:24 PM Matt Farina ***@***.***> wrote:
@philips <https://github.com/philips> I'm most interested in the easiest
solution that the lawyers will accept.
One issue is that a bot changing the repo means the local users commits
are different from the ones on the PR. This is going to create a different
experience to force pull (will that work?) to replace your local commits
with the bot altered commits on the same branch github is tracking against
the PR.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#83 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AACDCDAhzyQz3uACwbfxCEXAgrHJOnBhks5uVZkAgaJpZM4Vkph7>
.
|
@philips For these cases my concern is with the CNCF. That's why I asked @caniszczyk what the CNCF thinks of this. They can consult with the lawyers there. I tend to let Chris and Dan handle communicating with the CNCF lawyers. |
Just popping in from the perspective of the maintainers of this app. We just want it to work in a way that solves all of these use cases and would be super excited to make it easier for new folks to sign-off; however, we do want a +1 from the biggest users of the app before making such changes that perhaps lawyers might not like. 🙂 |
Thanks @mattfarina and @hiimbex. I guess it is back in @caniszczyk court to ask a Dolan or someone else. |
How do people currently deal with people editing files directly via the github.com web interface? |
@gundalow some people have hacked things using a chrome extension: https://chrome.google.com/webstore/detail/dco-github-ui/onhgmjhnaeipfgacbglaphlmllkpoijo?hl=en-US
|
Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? |
@caniszczyk , for the record - #105. I'm not sure if you're still open for that idea. |
Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? |
There are many times when a new contributor summits a PR and forgets to do the signed off by.
It would be great if the bot could allow commits associated with the PR to be rewritten by allowing the contributor to comment: "please sign off my commits" or something to that nature
The text was updated successfully, but these errors were encountered: