-
Notifications
You must be signed in to change notification settings - Fork 10
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 a GitHub Action that auto-checks the existence of Signed-off-by
#5
Comments
Signed-off-by
line. For the open source projects, Actions are free anyway so we maybe even add the Action to the template.Signed-off-by
@mbr0wn FYI I made an Issue for this |
Here's one popular GitHub Action that does this enforcement: https://github.com/probot/dco
However, it does not appear that GitHub supports signing commits from the web UI (todogroup/gh-issues#50), which would make any quick web contributions impossible. Worse, if you forget to sign your commit locally (e.g. committing in VS or VS Code doesn't put this option in the forefront), you'll need to interactive rebase all your commits while signing at each step... also annoying :) Aside: Here's the developer workflow for signing commits in VS CodeYou can auto-signoff everything via: microsoft/vscode#83096 Or you can commit with the signoff option: None of these are intuitive... you'd need to know there was a config token you could set for your workspace to take advantage of it. In order to persist this config token, you'd need to save the workspace.yml file to disk, which you may not want to do and have to remember not to clean out. The signoff option isn't any better: it's hidden in a menu and requires two clicks to use; the checkmark or the even more convenient and default Could we do it another way?
|
This is exactly what I changed in #4. I would suggest not doing that (hence my PR), as it is not tied to the repo. |
@benmont , could I get some background on why this signed-off-by is recommended (required?) by our NI template? Is it okay to not be using that format in repos that otherwise follow this template? It seems like it's not super common, and, for example, the data-record-ad-internal repo hasn't been enforcing it. We're not making large changes to Linux kernels, or even application software, rather writing plugins. Is the author field not sufficient traceability? @rdecarreau had the same question :) |
@mbr0wn ^ |
@dkapulkin @rdecarreau et al: When we create a new OSS project, we have to decide how we want to handle contributions from a legal perspective. There's basically three options:
The first option ("don't care") is something that some smaller open source projects do, but is obviously no option for us. That leaves DCO or CLA. If you go the DCO route, then you need the You can decide to go the CLA route instead, which makes a lot of sense, e.g., if we want to retain copyright on all contributions (DCO does not give you that). However, that means extra work when setting up the repository (as in, working with legal to draft the actual CLA document, defining the contribution requirements, etc.). If you do that extra work, then you don't need the DCO because contributors need to sign a separate document. There's at least one repo at NI that I know of that does that (https://github.com/EttusResearch/uhd/, see, e.g., EttusResearch/uhd#554). In the past, contributors had to download, print, sign, scan, and return to us a PDF document. Now, we use a bot do the signing). However, even if you go this route, there's no harm in keeping the signed-off-by requirement. It acts as an extra reminder that people had to sign something. CLAs are summarized here: https://nio365.sharepoint.com/sites/SoftwareDiscipline/SitePages/Open-Source-Tactics.aspx I assume you've seen the NI Open Source License Compliance Training: https://nio365.sharepoint.com/:p:/r/sites/OpenSource824/_layouts/15/Doc.aspx?sourcedoc=%7BBAC426C5-79E8-4EC2-B15F-B7AF1A5FA85E%7D&file=NI%20Open%20Source%20Licensing%20Training.pptx&action=edit&mobileredirect=true&cid=50e41383-0042-49db-9c77-0b856ebad9d7 |
@mbr0wn , thank you very much! I had not done that entire training yet, it was very useful. I'll check whether our Joint Developer Agreement with the external collaborators contained a CLA. I know right now our LICENSE file contains the default DCO. Should our developers be signing every commit or is it sufficient to sign just the squash commit of a PR going into main? In data-record-ad-internal we follow a branch and merge flow instead of forking so any given branch in the repo will have many commits. |
@dkapulkin Technically, you don't need to sign anything, because as NI employees you've implicitly signed off. NI owns your IP (unless you develop it out of hours, on your own gear, and it's unrelated to your work at NI, etc.). So you can choose a workflow that works best for you. I recommend that at least all commits on |
Just wanted to point out this was a bad choice of words and I've edited my comment above. I meant to emphasize that you can't accidentally get this wrong as an NI employee (when contributing to an NI-open-source-repo), because you don't own the IP in the first place. |
I understood the intent, no worries :D Our collaborators on our specific repo are operating as contractors under Joint Developer Agreement, so it sounds like they're already subject to the same terms (I can confirm with the team that drew up all the contracts). In either case it doesn't sound like we've broken any rules yet, and I now know what to do when we transition some IP to our public-facing repo. Thanks again for all the help, and sorry if the discussion on this PR wasn't the most appropriate place for this discussion! |
I like the idea of adding the Action to the template.
Originally posted by @benmont in #4 (comment)
The text was updated successfully, but these errors were encountered: