Turn off CI for forks

[Eric-Arellano]

Summary

Most people agree that GitHub made a mistake by having forks run CI both on the fork and in the upstream repository. It wastes computing resources (which has a real environmental cost!).

Details and comments

If people really want to run CI in their fork, they can always modify their fork's config to revert this change.

[coveralls]

Pull Request Test Coverage Report for Build 4929848047

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 75.793%

---

Totals
Change from base Build 4912573403:	0.0%
Covered Lines:	645
Relevant Lines:	851

---

💛 - Coveralls

[woodsp-ibm]

I was wondering whether we could use an env var here so this can be set in one place even if the tests need to be in multiple places ie have the check can refer to the env var. I know in the past we have tested things out on local forks - though I have actions disabled at present so things don't run. But if I wanted to run this it would mean I only needed to change one place instead of multiples.

[Eric-Arellano]

I was wondering whether we could use an env var here so this can be set in one place even if the tests need to be in multiple places ie have the check can refer to the env var.

Hm, like put the skip in the test code itself?

I wish that GitHub had a way for us to add an if on the entire workflow, rather than each job. But it unfortunately does not.

I know in the past we have tested things out on local forks

What was the motivation for this?

[woodsp-ibm]

Like setup an env var that is

env:
  REPO_OWNER: Qiskit

and do in the tests

if: github.repository_owner == REPO_OWNER

Not sure about quotes or not for string

I know in the past we have tested things out on local forks

What was the motivation for this?

In developing/changing workflows to test/debug them on a local fork before updating the main repo

[Eric-Arellano]

if: github.repository_owner == env.REPO_OWNER should be technically feasible. Although it adds a new form of complexity: there is more indirection to understand what is happening. For example, a reader must figure out what env.REPO_OWNER is all about.

That is, the suggestion solves one problem but introduces another.

Unless it is frequent that people need to re-enable CI checks on their fork rather than using upstream, I encourage going with the simpler solution of if: github.repository_owner == "Qiskit"

[woodsp-ibm]

It was more for us - already some 3rd party will need to figure things and look into the workflow and understand it to find out why its not running if that was what they expected. I would rather change it in one place if I want to run/modify it on my fork than in multiple places.

I think if they can do that they can figure what this is and the env var that affects it

if: github.repository_owner == $REPO_OWNER

After all the main concern was around wasting resources on peoples forks, not whether or not they can understand the workflow and if we are making things more complex in that regard,

[Eric-Arellano]

After all the main concern was around wasting resources on peoples forks, not whether or not they can understand the workflow and if we are making things more complex in that regard,

You're right; our primary goal is to minimize resource wastage. However, we also need to consider the code's maintainability and ease of understanding.

There are only four references in a single file: a simple solution is to use "Find all and replace".

Sometimes adhering strictly to DRY (Don't Repeat Yourself) is not the best approach, and following the WET (Write Everything Twice) principle is more appropriate: https://betterprogramming.pub/when-dry-doesnt-work-go-wet-6befda0444bf.

In this particular case, I suggest that we prioritize simplicity and maintainability over introducing additional complexity, which makes the code harder to understand.

[woodsp-ibm]

For me things would be easier all round with single constant rather than having to know I need to do search and replace. And for me its easier to maintain - I would not rate code very highly that replicated the same constant everywhere. Having said that its easy enough to change going forwards I guess and rather than spend more effort discussing I can go ahead with this though I do not consider it ideal.