Skip to content
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

please remove the signoff requirement (DCO) #1548

Closed
ssbarnea opened this issue Nov 3, 2018 · 13 comments · Fixed by #2416
Closed

please remove the signoff requirement (DCO) #1548

ssbarnea opened this issue Nov 3, 2018 · 13 comments · Fixed by #2416
Assignees
Labels

Comments

@ssbarnea
Copy link
Member

ssbarnea commented Nov 3, 2018

Issue Type

  • Bug report

The DCO check is a huge PITA as it requires everyone to use a very hard un unreliable workflow. Even the Ansible core team is not using it and I do not think that is molecule is such a high-profile / sensitive tools that really needs such an extra protection.

Not only that the automating it requires adding aliases for git-commit but these are NOT working when git is called from a non shell, like any git GUI client.

I tried the DCO github extension myself on another project and removed it after less than a day.

@ssbarnea
Copy link
Member Author

ssbarnea commented Nov 4, 2018

Update: I was able to find a way to make the Sign-off to kida work by adding a default commit template as mentioned on https://stackoverflow.com/questions/15015894/git-add-signed-off-by-line-using-format-signoff-not-working/34687806

Still, I think that if we still have to require the DCO, we need to clearly document how to configure it, and not by asking user to add aliases, as they work only on a small number of conditions.

Git template seems more generic as based on my tests it seems that most git clients do respect them.

@webknjaz

This comment has been minimized.

@gundalow gundalow removed the wontfix label Nov 5, 2018
@ssbarnea
Copy link
Member Author

ssbarnea commented Nov 9, 2018

Should I also mention that creating PRs from GitHub interface triggers the same DCO error?

See #1574 which was created by using github edit inteface. I am sure that I would not be the first to try this.

See dcoapp/app#83 -- not sure if I should laugh or cry... DCO bot is about to make the GDPR popups seems like nothing, probably next step would be to require signing a paper in blood and sending it by post, and even this would probably not prove safe-enough.

@MarkusTeufelberger
Copy link
Contributor

This came up in #1892 too, probably the discussion should be continued here.

@MarkusTeufelberger
Copy link
Contributor

I think it's a legal requirement.

It might be a requirement by the RedHat legal team, but it is not a legal requirement. Legally (in the sense of: in front of a judge/mandated by law) it doesn't matter if a certain string shows up in a commit message or not. Also I could just add the string Signed-off-by: John Doe <jdoe@example.com> to any commit manually, the DCO doesn't mandate that I sign off with my actual name...

@jelaplan
Copy link

I found this project when looking into whether others use this DCO bureaucratic nonsense. The statement "We'll keep it" is just so icky, yucky, gross. It things are a PITA then so what, you loser users can get over it.

@ssbarnea
Copy link
Member Author

@gundalow Can we sort this our once for all because the only outcome of the DCO implementation here is nothing else than a deterrent for getting contributions to the project. I prefer to avoid starting conspiratorial theories but if someone would want to kill a project, that is how it would start.

Here we are almost one year later with nothing changed and i do know very well that more sensitive projects like Ansible itself does not require it and that the check itself does not have any legal value because it can be forged.

PS. I am commenting again because I got 2 private messages about this in the last month.

@tehsmyers
Copy link
Contributor

Ansible itself does not require it

^ this is all that really matters for me, as a contributor. It seems odd to me that molecule would require the DCO when ansible itself does not.

@bgavrilMS
Copy link
Contributor

+1 it's just a hindrance

@MarkusTeufelberger
Copy link
Contributor

As an additional data point: https://github.com/ansible/awx seems also to have an DCO requirement for every single commit, but looking at recent commits nobody seems to actually care for some reason. Maybe contributors there assume that this doesn't apply for code that they write during work hours.

@decentral1se
Copy link
Contributor

We've got enough opinions on this. Locking until resolved further up the food chain.

@ansible ansible locked as resolved and limited conversation to collaborators Sep 16, 2019
@ssbarnea
Copy link
Member Author

@gundalow told me he will be looking at this after the fest. Trust me that I want this fixed, I will not forget about it.

gundalow added a commit to gundalow/molecule that referenced this issue Oct 27, 2019
fixes ansible#1548 ansible#1988

Disable DCO. Molecule is viewed to be "small" by certain metrics.

If/when GitHub makes it easier to do signoff's via the UI we may revisit this

GitHub app will also need disabling (that will be done after further review), this PR just tidies up the codebase.

Signed-off-by: John Barker <john@johnrbarker.com>
@helpr helpr bot added pr-available and removed pr-available labels Oct 27, 2019
decentral1se added a commit that referenced this issue Oct 28, 2019
* WIP Disable DCO

fixes #1548 #1988

Disable DCO. Molecule is viewed to be "small" by certain metrics.

If/when GitHub makes it easier to do signoff's via the UI we may revisit this

GitHub app will also need disabling (that will be done after further review), this PR just tidies up the codebase.

Signed-off-by: John Barker <john@johnrbarker.com>

* Improve language

Co-Authored-By: decentral1se <lukewm@riseup.net>
@helpr helpr bot added pr-merged and removed pr-rejected labels Oct 28, 2019
@gundalow
Copy link
Contributor

I've disabled DCO for Molecule.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants