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

adopting v2.4.9 and removing github checks #761

Merged
merged 1 commit into from
Jun 10, 2024
Merged

adopting v2.4.9 and removing github checks #761

merged 1 commit into from
Jun 10, 2024

Conversation

johndietz
Copy link
Contributor

No description provided.

@johndietz johndietz merged commit 310aa2e into main Jun 10, 2024
2 checks passed
@johndietz johndietz deleted the 2-4-9 branch June 10, 2024 05:49
@mrsimonemms
Copy link
Contributor

Context for this PR from @johndietz

i temporarily renamed these to .md until @fharper and i could talk. apparently he added these so that we could have checks for signed commits and terraform format. this is fine to have on most repos, but our gitops-template repo is the repo that produces the gitops repo. so this was basically causing every user to get errors in all of their gitops pull requests because their repos weren’t set up for signature verifications the same way our gitops-template repo was.
i don’t think this is a check that we want to flow down to our end users, so we’ll have to add a way to filter these files out during the construction of the gitops repo, if we’re dedicated to keeping them in the gitops-template repo.

@fharper
Copy link
Contributor

fharper commented Jul 8, 2024

Let's discuss this as the PR adding it was approved by John, and my question unanswered before it was, so I assumed it was OK.

CleanShot.2024-07-08.at.10.03.18.mp4

Since I now know it's passed to the user when we forked (I thought we were doing selective forking of the repository since we fork only what's needed depending on the Git provider and the cloud chosen by the user), there are two ways to go from here since we need to keep those for contributions (internal and external) to this repository (there a lot more checks I want to add also):

  1. We update the fork process to remove those actions so the users don't get them.
  2. We keep it as it is, and mention in the docs that it's best practices to sign the commits (with the reasons), and that if they don't want it, they can always deactivate it like everything we "force" to the user in our platform.

I'm all for #2 solution as we always said our gitops repository should show best practices. I also saw no complains from any users in the community when we had those, and I check them all. Lastly, it's common for any organization with good practices to sign their commits, and same goes for open source contributors. Not everyone do it though, so if we get too many complains about people not reading the docs if they get a signed issue (the action comment in the PR is pretty clear on the easy process of signing, and anyone using GitHub should probably have the knowledge to deactivate an action, or find it easily on GitHub docs or in our docs if we go with #2).

We could also just remove them, but that would make our internal process worse.

With that said, it is probably more about the fact that we have kbot unsigned commits (which I just remembered after typing everything else before this sentence 😅). So in that case, #2, would still be the best way to go, but we should fix konstructio/kubefirst#1680 first (which isn't that complicated to do: I was on my way to do it at https://github.com/kubefirst/docs/blob/main/.github/workflows/optimize-images.yml)

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