Skip to content

Conversation

@m1kola
Copy link
Member

@m1kola m1kola commented Jun 14, 2024

Description

It looks like #888 we moved rbac manifests, but did not update controller-gen output path. This PR should fix this.

Reviewer Checklist

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

Signed-off-by: Mikalai Radchuk <[email protected]>
@m1kola m1kola requested a review from a team as a code owner June 14, 2024 14:15
@netlify
Copy link

netlify bot commented Jun 14, 2024

Deploy Preview for olmv1 ready!

Name Link
🔨 Latest commit 0cc3799
🔍 Latest deploy log https://app.netlify.com/sites/olmv1/deploys/666c5074abee1c00089c3152
😎 Deploy Preview https://deploy-preview-937--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 site configuration.

@trgeiger
Copy link
Contributor

Thanks for catching this, LGTM

@codecov
Copy link

codecov bot commented Jun 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.16%. Comparing base (edc91ad) to head (0cc3799).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #937      +/-   ##
==========================================
- Coverage   79.34%   79.16%   -0.19%     
==========================================
  Files          16       16              
  Lines        1104     1104              
==========================================
- Hits          876      874       -2     
- Misses        158      159       +1     
- Partials       70       71       +1     
Flag Coverage Δ
e2e 57.60% <ø> (-0.55%) ⬇️
unit 53.71% <ø> (ø)

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.

@grokspawn
Copy link
Contributor

should we also .gitignore config/rbac since it is generated?

@m1kola
Copy link
Member Author

m1kola commented Jun 14, 2024

@grokspawn nothing should write into config/rbac anymore. Not sure that ignoring it makes a lot of sense.

Copy link
Member

@varshaprasad96 varshaprasad96 left a comment

Choose a reason for hiding this comment

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

Agree with @m1kola. We aren't adding other generated files to gitignore, and should keep it the same for rbac.

/lgtm
/approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 14, 2024
@grokspawn
Copy link
Contributor

@grokspawn nothing should write into config/rbac anymore. Not sure that ignoring it makes a lot of sense.

Ah! I had leftover artifacts there after running the test for you this morning, and presumed that path was still used. If vestigial, ignore!

@m1kola m1kola added this pull request to the merge queue Jun 14, 2024
Merged via the queue into operator-framework:main with commit 126ddc4 Jun 14, 2024
@m1kola m1kola deleted the fix_rbac_generation branch June 14, 2024 14:54
@skattoju skattoju mentioned this pull request Sep 25, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants