Skip to content

ci: Remove DCO Checks from all InstructLab repositories #192

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

Merged
merged 1 commit into from
Jun 2, 2025

Conversation

courtneypacheco
Copy link
Contributor

This document proposes the removal of explicit DCO checks from all InstructLab repositories. If explicit DCO checks are removed, then contributors no longer have to "sign off" each commit.

Copy link
Contributor

@ktdreyer ktdreyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Courtney! I support this idea because

  • Several editors (like GitHub's web UI) do not add these lines to commit logs.
  • New contributors do not always have advanced Git knowledge to amend commit messages.
  • The DCO Bot sometimes has outages, delaying merging PRs.

@nathan-weinberg nathan-weinberg requested a review from a team February 20, 2025 16:22
@jjasghar
Copy link
Member

I thought the DCO thing was from Red Hat/IBM Legal. I strongly suggest we get legal entities to sign off on this before we continue moving forward.

Copy link
Member

@jjasghar jjasghar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment above.

@booxter
Copy link
Contributor

booxter commented Feb 25, 2025

I believe legal did sign off on this, but @ktdreyer could share specific snippet I guess.

@ktdreyer
Copy link
Contributor

I've checked with RH Legal on this and they've agreed to this change.

@jjasghar
Copy link
Member

Not that I don't trust you, but I we need some level of paper trail with this. @lhawthorn can you advise here? I'm not comfortable signing off on this as an OC committee member without seeing the paper trail.

@lhawthorn
Copy link
Member

It is a requirement from the legal team. Following up via real time chat, will summarize outcome in this PR.

@lhawthorn
Copy link
Member

Hi Folks, after a long session of hashing out in real time chat about legal requirements and the DCO, I am asking that we keep DCO for any submissions to the taxonomy repo, as removing it has significant implications for the community model rebuild work.

I leave it to other folks who are focused on the workflow for InstructLab mechanics vs. contributions to the community model build to understand if there are further implications to removing the DCO from other repos, but I do not immediately identify them.

Pinging also @instructlab/ui-maintainers because the DCO is part of their UI workflow and this change will possibly have implications for them.

@markstur
Copy link
Member

  • Several editors (like GitHub's web UI) do not add these lines to commit logs.

The org has sign-off via web ui enabled so it should be just a checkbox.

@markstur
Copy link
Member

There is an alternative CLA process so that contributors agree that they have agreed to the contributor agreement. It is much worse than DCO sign-off.

@vishnoianil
Copy link
Member

Pinging also @instructlab/ui-maintainers because the DCO is part of their UI workflow and this change will possibly have implications for them.

UI has DCO workflow because taxonomy requires DCO, even if taxonomy doesn't require it in future, we can still keep signing-off the commits, because UI does the signoff on behalf of the user. To contribute to the UI project itself, it can adopt any guidelines.

In my humble opinion, signoffs and CLAs are considered best practices. Signoff enforces that user have a Name on the profile and primary email address. Incase user is unresponsive (may be filtering github notifications) we can reach out through email address if requires. It gives an alternate communication channel apart from github comments.

@markstur
Copy link
Member

Pinging also @instructlab/ui-maintainers because the DCO is part of their UI workflow and this change will possibly have implications for them.

UI has DCO workflow because taxonomy requires DCO, even if taxonomy doesn't require it in future, we can still keep signing-off the commits, because UI does the signoff on behalf of the user. To contribute to the UI project itself, it can adopt any guidelines.

In my humble opinion, signoffs and CLAs are considered best practices. Signoff enforces that user have a Name on the profile and primary email address. Incase user is unresponsive (may be filtering github notifications) we can reach out through email address if requires. It gives an alternate communication channel apart from github comments.

I think it's not just so they have a name/email, but it is (somehow) implied that by signing the commit they are agreeing to the terms of the DCO, which gives us the rights to the code, and states that they had the rights to give it to us. It's like signing a contract (maybe w/o reading the contract). No take backs.

I'm definitely not a lawyer though.

Copy link

This pull request has been automatically marked as stale because it has not had activity within 30 days. It will be automatically closed if no further activity occurs within 7 days.

@github-actions github-actions bot added the stale label Apr 28, 2025
Copy link

github-actions bot commented May 5, 2025

This pull request has been automatically closed due to inactivity. Please feel free to reopen if you intend to continue working on it!

@github-actions github-actions bot closed this May 5, 2025
@RobotSail RobotSail reopened this May 5, 2025
@RobotSail
Copy link
Member

So it sounds like we are all good to remove DCO, just taxonomy needs to keep it. @jjasghar it looks like you have requested changes, is there anything else you feel is missing currently?

@github-actions github-actions bot removed the stale label May 6, 2025
@ktdreyer
Copy link
Contributor

ktdreyer commented Jun 2, 2025

This blocked another contributor's PR recently instructlab/training#578 and I think we should merge this and implement it.

@RobotSail
Copy link
Member

I agree @ktdreyer , it seems like we can go ahead and merge this since I haven't heard any objections since the last message.

@booxter booxter dismissed jjasghar’s stale review June 2, 2025 20:10

The contributor had enough time to express concerns. If these are still valid, feel free to raise a new issue or revert the patch.

@booxter booxter merged commit 6a660e7 into instructlab:main Jun 2, 2025
6 checks passed
@booxter
Copy link
Contributor

booxter commented Jun 2, 2025

@courtneypacheco @ktdreyer do you plan to take care of the execution here? (Putting DCO.txt per repo etc.)

@ktdreyer
Copy link
Contributor

ktdreyer commented Jun 2, 2025

I can do this for the four repos where this will have the most immediate impact: instructlab core, eval, sdg, and training.

@ktdreyer
Copy link
Contributor

ktdreyer commented Jun 4, 2025

@booxter you were right in your earlier comments about dropping the mergify config. We do need to drop the mergify gates prior to updating our DCO settings for our instructlab org to shut the job off altogether.

I'll push PRs to do that now.

mergify bot added a commit to instructlab/instructlab that referenced this pull request Jun 4, 2025
Add the contents of https://developercertificate.org/ to `DCO.txt` at the root of this repository.

Explain to contributors that new submissions imply acceptance of the DCO.


Related: instructlab/dev-docs#192



Approved-by: booxter

Approved-by: jwm4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants