Do we need to run actions on forks? #7066
Description
Right now each my PR is tested twice: on python/typeshed@issue-branch and sobolevn/typeshed@issue-branch. Do we really want this?
In case we want to switch it off, it is as simple as:
on:
workflow_dispatch:
push:
branches: [master]
pull_request:
branches: [master]Here: https://github.com/python/typeshed/blob/master/.github/workflows/tests.yml#L5
I can send a PR if needed 🙂
Activity
hauntsaninja commentedon Jan 28, 2022
There have been times where I've found actions on my fork useful (although not so much these days). But I could also just open a draft PR to typeshed. Also you could turn off actions for your fork at https://github.com/sobolevn/typeshed/settings/actions So there's a lot of choices!
sobolevn commentedon Jan 28, 2022
In my point of view, disabling PRs on forks by default is a good thing!
Let's wait for other opinions 🙂
Akuli commentedon Jan 28, 2022
I like this. If we decide to disable, and someone wants it back, they can just edit the yml file in their fork.
Also, I have never seen
branches: [master]underpull_requestbefore. It works without it too.hugovk commentedon Jan 30, 2022
I'm personally strongly against this, editing the yml in their fork, and then having to remember to revert it before creating a PR, is an extra impediment to contributing.
Or I can submit a PR on my branch first to check it passes -> extra impediment.
Or I can develop on my
master-> not a best practice, we should use feature branches, and I only have a singlemaster-> extra impediment.The fact it's tested on upstream and on the fork isn't such a big deal: the testing is split between two different accounts and therefore doesn't use up any extra quota/time from any single account.
CIs allow us to test on a much larger matrix of Python versions and operating systems, much more than is possible locally and sometimes faster too. I sometimes avoid contributing to projects with no/broken CI.
I believe the use of CIs are of huge benefit for software quality and development, especially when so easy nowadays; Travis CI really revolutionised this, remember how hard testing was before. We should rather encourage people to test on their own forks before creating PRs.
Fail fast: shorten the time to finding a failure, enable quicker loops of incremental development.
nightlark commentedon May 30, 2022
Most of the concerns are non-issues since there exists an alternative due to the
workflow_dispatchtrigger allowing you to manually to trigger CI runs on any branch in a fork. See below screen capture for how to access the feature.Editing the yaml file is not necessary, unless your branch was created based off an old main/master branch that pre-dates the addition of the
workflow_dispatchtrigger. In which case you should probably rebase the branch or update it to be more in sync with main/master; in which case reverting would not be required.Using the
workflow_dispatchtrigger doesn't require opening any PRs. It also doesn't require making any spammy commits.You can develop on a feature branch with any name you want, it will show up in the list of branches you can select when triggering the workflow.
The biggest downside to automatically running multiple tests is if you make multiple commits one after another making minor changes, then all of them will trigger a potentially large matrix of tests -- last I remember, each account has a concurrency limit of 20 running jobs across all personal repositories.
hugovk commentedon May 30, 2022
They're non-non-issues ;)
Sure, it's possible to trigger via
workflow_dispatch, but it's still an extra impediment. Making the human do more work so the computer can do less.As shown in your screencast, it can take 6 clicks and 15 seconds to trigger each and every build.
That's quite a bit of tedious manual labour.
Automated tests should be automatic :)
AlexWaygood commentedon May 30, 2022
Hi! I think we're pretty happy with our CI over here at typeshed. This issue has been closed for a while, and our CI situation has changed in the meantime because we were running out of CI minutes on a regular basis (#7883).
I'm sure there are further improvements we could make, but I'm not sure it's helpful to have a proxy debate here, on a closed issue, for a disagreement that's happening over on the bedevere issue tracker. If anybody wants to propose further changes to our CI, they're of course welcome to open a new issue or PR.