Skip to content

Conversation

@Carreau
Copy link
Member

@Carreau Carreau commented Feb 12, 2025

It says its for branch protection, but branch protection can already requires multiple checks, and it creates an arbitry extra failure, which is anyway ignored as downasrteam is failing on main.

So it's just extra noise IMHO.

It says its for branch protection, but branch protection can already
requires multiple checks, and it creates an arbitry extra failure, which
is anyway ignored as downasrteam is failing on main.

So it's just extra noise IMHO.
@Carreau Carreau added this to the 7.0 milestone Feb 12, 2025
@Carreau
Copy link
Member Author

Carreau commented Feb 12, 2025

Actually I'm going to a say that IMHO using downstream check is even worse that using via protected banches, as now you have no direct visibility from the PR to which check are required and which hare not:
Screenshot 2025-02-12 at 15 06 03

You now get an arbitrary indirection, a link you must click, and scroll through to see what is actually needed...

@minrk
Copy link
Member

minrk commented Feb 13, 2025

👍 I removed the downstream_check branch protection rule

@Carreau Carreau merged commit 521549a into ipython:main Feb 14, 2025
27 of 32 checks passed
@Carreau Carreau deleted the remove-down-noise branch February 14, 2025 07:24
Carreau added a commit to Carreau/ipykernel that referenced this pull request Feb 17, 2025
Same as ipython#1318, there is no reasons for it to be there anymore, and it is more
annoying than anything else. It used to be usefull if you wanted to enforce some
CI items but not other, but this is now native via branch protection rules, but
now Github UI shows which jobs are required to pass for a PR to be merged, and
with this it does not.
@Carreau Carreau mentioned this pull request Feb 17, 2025
Carreau added a commit to Carreau/ipykernel that referenced this pull request Feb 17, 2025
Same as ipython#1318, there is no reasons for it to be there anymore, and it is more
annoying than anything else. It used to be usefull if you wanted to enforce some
CI items but not other, but this is now native via branch protection rules, but
now Github UI shows which jobs are required to pass for a PR to be merged, and
with this it does not.
ianthomas23 pushed a commit to ianthomas23/ipykernel that referenced this pull request Jul 14, 2025
ianthomas23 pushed a commit to ianthomas23/ipykernel that referenced this pull request Jul 15, 2025
@ianthomas23 ianthomas23 mentioned this pull request Jul 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants