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

🌱 (fix) Revert: Check cluster is running for non-standalone Make targets #461

Closed
wants to merge 2 commits into from

Conversation

camilamacedo86
Copy link
Contributor

@camilamacedo86 camilamacedo86 commented Nov 18, 2024

Reverts #437

If we see the logs https://github.com/operator-framework/catalogd/actions/runs/11819448861/job/32929383546 (ci job of PR where the change was introduced), we can see that this check is wrong and is always failing:

Screenshot 2024-11-18 at 08 16 07

PS.: I don't think there's a need for this check. Proof of that is that besides it does not work, nothing changed. Therefore, I do not see value in this one.
Instead, I would prefer the targets of this project to be as similar as possible to those of https://github.com/operator-framework/operator-controller so that life for us contributors is easier.

@camilamacedo86 camilamacedo86 requested a review from a team as a code owner November 18, 2024 08:20
@camilamacedo86 camilamacedo86 requested review from bentito and grokspawn and removed request for a team November 18, 2024 08:20
@camilamacedo86 camilamacedo86 changed the title Revert "🐛 Check cluster is running for non-standalone Make targets" 🐛 Revert: Check cluster is running for non-standalone Make targets Nov 18, 2024
Copy link

codecov bot commented Nov 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 38.23%. Comparing base (55689e8) to head (9fddb33).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #461   +/-   ##
=======================================
  Coverage   38.23%   38.23%           
=======================================
  Files          15       15           
  Lines        1224     1224           
=======================================
  Hits          468      468           
  Misses        706      706           
  Partials       50       50           

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

@camilamacedo86 camilamacedo86 changed the title 🐛 Revert: Check cluster is running for non-standalone Make targets 🌱 (fix) Revert: Check cluster is running for non-standalone Make targets Nov 18, 2024
perdasilva
perdasilva previously approved these changes Nov 18, 2024
@perdasilva perdasilva added this pull request to the merge queue Nov 18, 2024
@joelanford joelanford removed this pull request from the merge queue due to a manual request Nov 18, 2024
@camilamacedo86 camilamacedo86 added this pull request to the merge queue Nov 18, 2024
@joelanford
Copy link
Member

I removed from the queue. Just wanted to discuss this a bit more because:

  • the tests were passing when the PR merged. Are they failing now?
  • Brett put quite a bit of effort into this and has good motivations. We should discuss what changes should be made to remediate problems.

@joelanford joelanford removed this pull request from the merge queue due to a manual request Nov 18, 2024
@joelanford
Copy link
Member

@camilamacedo86 can you point to a where the e2e or upgrade-e2e tests failed as a result of #437?

@camilamacedo86
Copy link
Contributor Author

camilamacedo86 commented Nov 18, 2024

Hi @joelanford

@camilamacedo86 can you point to a where the e2e or upgrade-e2e tests failed as a result of #437?

See the description: #461 (comment)
The check fails always; it is wrong since it was added
That does not mean that the tests fail.

@joelanford
Copy link
Member

That job didn't fail.

@camilamacedo86
Copy link
Contributor Author

camilamacedo86 commented Nov 18, 2024

HI @joelanford

That job didn't fail.

Not because. As far as I can see, the check is wrong and not helpful for anything.
Just bring the noise in the command line every time we run.

@camilamacedo86
Copy link
Contributor Author

It is closed in favour: #465
I missed the rebase with the master
Then, I need to create a local branch to do so.

@camilamacedo86 camilamacedo86 deleted the revert-437-checking_kind_running branch November 18, 2024 14:32
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