Skip to content

Conversation

@nfebe
Copy link
Contributor

@nfebe nfebe commented Oct 30, 2025

The existing /compile amend command removes [skip ci] but merges the compile step with the changes themselves which is not ideal to maintain a cleaner history. Sometimes GitHub reviews are sufficient to confirm a backport without need to pull and ammend locally in the case where the developer prefers not to squash the compile step.

This new command allows removing [skip ci] from commits without triggering asset compilation.

@nfebe nfebe requested a review from a team as a code owner October 30, 2025 12:19
@nfebe nfebe requested review from icewind1991, salmart-dev and yemkareems and removed request for a team October 30, 2025 12:19
@nfebe nfebe changed the title feat: add /remove-skip-ci command for backport workflows chore: add /remove-skip-ci command for backport workflows Oct 30, 2025
The existing /compile amend command removes [skip ci] but merges the
compile step with the changes themselves which is not ideal to maintain
a cleaner history. Sometimes GitHub reviews are sufficient to confirm a
backport without need to pull and ammend locally in the case where the developer
prefers not to squash the compile step.

This new command allows removing [skip ci] from commits without
triggering asset compilation.

Signed-off-by: nfebe <[email protected]>
@nfebe nfebe force-pushed the add-remove-skip-ci-command branch from 9cfda10 to 8bf8b01 Compare October 30, 2025 12:25
@skjnldsv
Copy link
Member

skjnldsv commented Oct 30, 2025

If skip-ci is here, it means there was conflicts.
There are two options here:

  1. it's a javascript assets conflicts, in which case /compile should be enough to trigger the ci again with clean github history
  2. it's something else, in which case manual work is needed anyway, so devs should be removing the skip-ci tag manually

Or did I miss something else? :)

@nfebe
Copy link
Contributor Author

nfebe commented Oct 30, 2025

If skip-ci is here, it means there was conflicts.

True but there is code in the backportbot that attempts to resolve the conflicts which is sometimes effective.

Additionally in this function : https://github.com/nextcloud/backportbot/blob/master/src/gitUtils.ts#L83 It appears that we always try to backport all commits including chore (or compile commits) which often lead to conflicts that cannot be resolved by the auto resolve strategy in place.

The commit is always dropped for (compiled files as they conflict) but the hasConflicts is set to true for that reason, and the skip-ci flag is added. In this case it is not definite that the main backported code has any conflicts.

So it seems ALL PRs with compiled assets (which is always conflicting across versions) always have skip-ci but the main and important code is not always conflicting so local resolution is not always necessary.

Or did I miss something else? :)

Not hundred percent sure beyond what I explained above, my I definitely encounter commits with [skip-ci] that do not need any local handling a lot of times.

@susnux
Copy link
Contributor

susnux commented Nov 3, 2025

But the skip-ci is only added to the commit with conflicts.
So in that case always manual interaction of the author is required.
If the conflicts happens in the compile commit then you can just trigger /compile to resolve it.

(except when you commit compiled assets within the source commit, but thats not that nice anyways)

@nfebe
Copy link
Contributor Author

nfebe commented Nov 4, 2025

But the skip-ci is only added to the commit with conflicts.

It's added to the head commit if there is a conflict in any one commit. (Usually the compile step) which is then removed as it impossible to resolve, leaving a PR without a conflict with the marker behind.

@skjnldsv
Copy link
Member

skjnldsv commented Nov 4, 2025

leaving a PR without a conflict with the marker behind.

Seems like a fix for the bot then? https://github.com/nextcloud/backportbot

@nfebe
Copy link
Contributor Author

nfebe commented Dec 15, 2025

Here is an example of a PR with no conflict that gets marked due to a dropped/empty compile commit: #56974

Seems like a fix for the bot then? https://github.com/nextcloud/backportbot

Sounds right, will leave this open till I can link a fix from that end. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants