jajik commented on code in PR #272:
URL: https://github.com/apache/commons-daemon/pull/272#discussion_r2269141984
##########
.github/workflows/codeql-analysis-cpp.yml:
##########
@@ -17,7 +17,6 @@ name: "CodeQL CPP"
on:
push:
- branches: [ master ]
Review Comment:
> If the branch restriction is removed, **every** PR to `commons-daemon`
will cause the workflow to run twice:
>
> 1. **On `push`** — triggered when I push the branch to the repository.
>
> 2. **On `pull_request`** — triggered again when the PR is opened.
Well, yes and no. The 1. will run before a PR is opened in the repository
where the branch is. That may be a feature branch in `commons-daemon` but it
may be a fork as well. I would argue that it's better to know whether something
is broken before creating a PR (that will always run in `commons-daemon`).
> I understand the need to test workflow changes more flexibly, but there
are alternatives that avoid this duplication:
>
> * Temporarily remove the branch restriction in your **personal fork**
while working on the workflow.
>
> * Restore the restriction before opening the PR to
`apache/commons-daemon`.
>
>
> This way, you can still test changes freely without doubling the CI runs
on every PR from committers.
That's not that easy. To remove the branch restriction it's necessary to
modify the workflow file which means adding a new commit atop. When the
upstream adds a new commit, this workflow modifying patch must be discarded,
the new commits from the upstream consumed and then the discarded commit must
be added again. Then you can rebase a feature branch from which you must omit
the workflow patch. It's definitely doable, but imho it's not very friendly and
is unnecessary.
The question is why is the "duplication" a problem here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
