Run linting workflow only on pushes to main branch or pull requests.

[Shivansh-007]

We don't want to build, lint (and deploy - In Future) on every branches, and currently the Build works after Linting which runs on every pull request or push to master, hence even running the Build workflow.

The reasons for not running them are:

• They are run twice.
  • In case of forks: On the pull request and the branch (mainly people push from separate branches, and not main)
  • In case of main: On the branch and the pull request.

[gustavwilliam]

Not sure why we wouldn’t want to run it on branches. It doesn’t cost us anything, and I’d like us to have linted and working code on all branches.

Could you give a second opinion here @Akarys42?

[dawnofmidnight]

I agree. We want clean code everywhere. We don't want to have to deal with messy code later, it's best that it remain how it is in my opinion.

[Shivansh-007]

I agree with that, but there is no reason to build docker images on forks and other branches.

[dawnofmidnight]

I agree with that, but there is no reason to build docker images on forks and other branches.

I think doing it on all branches + all pull requests is best. Because there's no reason to do it on forks if they're not going to be used.

[Shivansh-007]

Why should it run on other branches? Docker images are mainly used for production use only and I don't see a reason to pull a branch, which is still in development.

[dawnofmidnight]

Do we need the full docker image though? What if we only use the docker image on main, and just lint everywhere else?

[vivekashok1221]

I agree with that, but there is no reason to build docker images on forks and other branches.

I guess it's to ensure that the build doesn't break when we use it in production, but I am not sure.

[Shivansh-007]

We have pull requests which are used to merge anything to main, and build action are ran on them.

[gustavwilliam]

What's that supposed to mean?

[Akarys42]

The problem with linking on pushes and pull requests is that if you push a commit that also have a pull request opened you’d lint it twice. While we are billed for that, I’d rather avoid it when possible.

That said, there are smarter ways like only linting commits on pushes if there aren’t on a PR (skip otherwise). I’d prefer to see that here, what do you think @gustavwilliam ?

[Shivansh-007]

The problem with linking on pushes and pull requests is that if you push a commit that also have a pull request opened you’d lint it twice. While we are billed for that, I’d rather avoid it when possible.

Well here it is, Akarys put it in better words than what I said in the PR description.

[gustavwilliam]

I don't quite follow. When do you mean that it would lint twice?

[Akarys42]

I don't quite follow. When do you mean that it would lint twice?

It would match both the push and the pull_request triggers, it will trigger a run for both of those. One push can run the same action more than once, like here.

[gustavwilliam]

Ah, I see, thanks. That does sound unnecessary.

It also seems like this PR won't solve that issue, right? I'm not sure what the best method would be, but I trust your judgment.

[Akarys42]

This PR solves this issue, assuming that we don’t open PRs from the main branch, although it will stop linting of branches without a PR open with is pretty sad. That’s why I think we should be smarter about it and skip the push trigger if there’s a PR opened for this branch, should be fairly easy to do.

[gustavwilliam]

I'm not in favor of stopping limiting on branches without a PR, just to fix that issue. Your proposed solution sounds much cleaner imo.

Could you open an issue/pr detailing what there is to be done? Maybe you, Jason or someone else would like to implement it?

[Shivansh-007]

@Akarys42 Are you willing to implement it?

[Akarys42]

I don’t have much free time right now but GitHub recently added a concurrency key to workflows which is designed for this.

Announcement blog: https://github.blog/changelog/2021-04-19-github-actions-limit-workflow-run-or-job-concurrency/