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

GitHub actions based DCO check #609

Closed
wants to merge 3 commits into from
Closed

Conversation

martinmo
Copy link
Member

@martinmo martinmo commented Jun 5, 2024

As a (maybe temporary) workaround for the currently malfunctioning DCO check.

Downside: this action has to be added to every repository in the org – in other words, this PR fixes only the standards repo.

The DCO app "required status check" must be disabled in the "Branches" settings of this repo.

Signed-off-by: Martin Morgenstern <martin.morgenstern@cloudandheat.com>
Signed-off-by: Martin Morgenstern <martin.morgenstern@cloudandheat.com>
@martinmo martinmo self-assigned this Jun 5, 2024
@martinmo
Copy link
Member Author

martinmo commented Jun 5, 2024

I've tested it with a non-conformant commit, which can be seen in the Actions log: https://github.com/SovereignCloudStack/standards/actions/workflows/dco-check.yml (this branch originally contained an empty commit without the Signed-off-by tag).

@martinmo martinmo marked this pull request as ready for review June 5, 2024 07:49
@berendt
Copy link
Member

berendt commented Jun 5, 2024

Is it likely that it will be broken for longer? Otherwise we have to check the sign-off manually for the moment. I think that's better than having to touch all repos or having a snowflake on individual repos. Just my 2 cents.

@martinmo martinmo requested review from fkr and garloff June 5, 2024 07:49
@martinmo
Copy link
Member Author

martinmo commented Jun 5, 2024

Is it likely that it will be broken for longer? Otherwise we have to check the sign-off manually for the moment. I think that's better than having to touch all repos or having a snowflake on individual repos. Just my 2 cents.

That's a good question! The DCO app doesn't seem to get a lot of attention these days, and there have been several outages in the past before the one this week, e.g., dcoapp/app#162.

This PR is just a proposal.

@martinmo
Copy link
Member Author

martinmo commented Jun 5, 2024

PS. Wrt to attention, the last release of the dco-check package which I use here is also 2 years ago :(

@berendt
Copy link
Member

berendt commented Jun 5, 2024

There is a DCO job for Zuul. Perhaps it would be better to use this?

https://zuul-ci.org/docs/zuul-jobs/latest/general-jobs.html#job-dco-license

@berendt
Copy link
Member

berendt commented Jun 5, 2024

This is the related issue: dcoapp/app#211

Looks like the bot is up & running again.

@garloff
Copy link
Contributor

garloff commented Jun 6, 2024

In general, I am in favor of merging this into just this one repository.
So we have a replacement for a broken github DCO check which is used here in real life.
Based on what happens with github's own DCO, we have thus an alternative in stock.

@fkr fkr removed their request for review June 6, 2024 08:41
@garloff
Copy link
Contributor

garloff commented Jun 6, 2024

Hmm, as DCO works again, we don't need this right now.
Should we close this and reopen, if the github folks continue to do a bad job and we see failure again?

@martinmo
Copy link
Member Author

I'll close it, we can revive it when needed.

@martinmo martinmo closed this Jun 10, 2024
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.

3 participants