Skip to content

🌱 migrate from github.com/everettraven/crd-diff to sigs.k8s.io/crdify - #2010

Closed
everettraven wants to merge 1 commit into
operator-framework:mainfrom
everettraven:ci/update-crd-diff-to-crdify
Closed

🌱 migrate from github.com/everettraven/crd-diff to sigs.k8s.io/crdify#2010
everettraven wants to merge 1 commit into
operator-framework:mainfrom
everettraven:ci/update-crd-diff-to-crdify

Conversation

@everettraven

Copy link
Copy Markdown
Contributor

Description

This updates the usage of github.com/everettraven/crd-diff in CI to sigs.k8s.io/crdify as the project has now been donated under the umbrella of sig-api-machinery.

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

@everettraven
everettraven requested a review from a team as a code ownerJune 4, 2025 18:30
@netlify

netlifyBot commented Jun 4, 2025

Copy link
Copy Markdown

Deploy Preview for olmv1 ready!

NameLink
🔨 Latest commit73ec45b
🔍 Latest deploy loghttps://app.netlify.com/projects/olmv1/deploys/685da59251e8770008987186
😎 Deploy Previewhttps://deploy-preview-2010--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Comment threadcrd-diff-config.yaml
Comment on lines +1 to +5
validations:
- name: enum
enforcement: Error
configuration:
additionPolicy: Allow

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Note for reviewers: This is technically a less restrictive config than existed before, but IIRC it was planned to add new enum values in various places during continued development of OLMv1 so this felt reasonable to include.

If you want to maintain the strictest possible configuration, the options here could be to:

  • Remove the use of a configuration file as crdify defaults to the strictest possible analysis
  • Switch additionPolicy to Disallow (keeps the config file around)

@codecov

codecovBot commented Jun 4, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.80%. Comparing base (a449fc4) to head (73ec45b).
Report is 8 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #2010 +/- ##
==========================================
- Coverage 73.82% 73.80% -0.02% 
==========================================
Files 81 81 Lines 7365 7365 ==========================================
- Hits 5437 5436 -1 - Misses 1588 1589 +1 
Partials 340 340 
FlagCoverage Δ
e2e44.10% <ø> (+0.04%)⬆️
unit60.24% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tmshorttmshort added the do-not-merge/new-go-version Changes that require a go update. label Jun 4, 2025
@perdasilva

Copy link
Copy Markdown
Contributor

@everettraven because this pulls in go1.24, we won't be able to merge it T_T

@everettraven

Copy link
Copy Markdown
ContributorAuthor

@perdasilva I noted that in a Slack message I sent to the olm-dev channel :)

I imagine that you'll be bumping to go 1.24 at some point? I'm happy for this to be held until that point. I don't think this is urgent

@camilamacedo86camilamacedo86 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks great — we just need to wait until we can use Go 1.24 to move forward with it. Everything else is looking solid 🎉

Thanks for coming back and contributing toward this!
/hold until there.

cc @grokspawn@tmshort — another one queued up for when we’re able to move to Go 1.24, just FYI for your ACK.

@openshift-ciopenshift-ciBot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 10, 2025
@openshift-ciopenshift-ciBot added the lgtm Indicates that a PR is ready to be merged. label Jun 10, 2025
@openshift-ci

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ciopenshift-ciBot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 10, 2025
@camilamacedo86

Copy link
Copy Markdown
Contributor

Hi @everettraven

Can you please rebase it with main branch and ensure that we use go 1.24.3 so that we can get this one merged?

Signed-off-by: Bryce Palmer <bpalmer@redhat.com>
@everettraven
everettravenforce-pushed the ci/update-crd-diff-to-crdify branch from cfb911f to 73ec45bCompareJune 26, 2025 19:54
@openshift-ciopenshift-ciBot removed the lgtm Indicates that a PR is ready to be merged. label Jun 26, 2025
@openshift-ci

Copy link
Copy Markdown

New changes are detected. LGTM label has been removed.

@everettraven

Copy link
Copy Markdown
ContributorAuthor

@camilamacedo86 done

@camilamacedo86

Copy link
Copy Markdown
Contributor

Hi @everettraven

Can you please rebase again ?
So that we can get it merged?

@openshift-merge-robotopenshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 27, 2025
@openshift-merge-robot

Copy link
Copy Markdown

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@perdasilva

Copy link
Copy Markdown
Contributor

closing this as stale - please re-open if its still important

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approvedIndicates a PR has been approved by an approver from all required OWNERS files.do-not-merge/holdIndicates that a PR should not merge because someone has issued a /hold command.needs-rebaseIndicates a PR cannot be merged because it has merge conflicts with HEAD.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@everettraven@perdasilva@camilamacedo86@openshift-merge-robot@tmshort