-
-
Notifications
You must be signed in to change notification settings - Fork 15k
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
rm .github/workflows/pending-* #100269
rm .github/workflows/pending-* #100269
Conversation
These actions mess with CI status for no obvious reason. They make it harder to tell a real eval failure from a pending build. The ofborg builder for darwin is not working currently, so this marks many PRs with a failed state.
this is the actual issue here #96707 |
They are needed for the editorconfig check.
Will be fixed by NixOS/ofborg#527.
No it does not. |
I would suggest removing it too. It affects a tiny fraction of pull requests, I would not be surprised if there is only a single digit number of people who actually consume this check. It makes no sense to me to keep it in the UI. |
https://github.com/NixOS/nixpkgs/actions?query=workflow%3A%22Checking+EditorConfig%22+is%3Afailure |
Regardless of editorconfig we'll need the pending job if we want to work on #99722. |
@zowoq What are you trying to say? If I'm not mistaken, following your link I see that the check has some spurious fails for unrelated PR's. |
They aren't spurious. |
My bad. I see now. |
It isn't related to darwin.
Not sure exactly what happened with this one (being closed/reopened without pushing changes may have caused it to glitch) but it wouldn't have happened if we had NixOS/ofborg#527 and #96707. |
The editorconfig check is important. Before it we had lots of trailing spaces. And if you had an editorconfig-aware editor, then it would automatically strip them. I've made several PRs with unrelated (accidental) whitespace changes in Maybe the editorconfig check should be a different actions workflow though. |
Maybe the editorconfig check should first be announced on the mailing list (along with other CI plans), and at the least documented what is expected here from contributors. I'm pretty sure this has been said several times. |
Speaking of EditorConfig, I'd like to suggest that it'll be more strict, also regarding spaces vs tabs (example). Also 👍 on whoever that decided it should notify users when this check fails. |
CLosing this based on the discussions. |
Apologies, seems I forgot about this PR.
Thought I'd already commented on this in another PR but if I did I can't find it. As this is the fourth attempt at this check I was waiting until we didn't need the weird workarounds.
Disallowing tabs was done in #101530. Indentation size is more disruptive, I'm still looking at ways to enable it.
Github, currently we aren't able to configure notifications. (IIRC users can disable them globally in their profile?) |
These actions mess with CI status for no obvious reason. They make it
harder to tell a real eval failure from a pending build. The ofborg
builder for darwin is not working currently, so this marks many PRs
with a failed state.
cc @zowoq