build(ci): Add missing '+ffmpeg' label to build job and steps by tsunamistate · Pull Request #3251 · TheSuperHackers/GeneralsGameCode

tsunamistate · 2026-09-03T11:58:01Z

Follow up to #3184

Due to me to not Reading the Freaking Manual enough, +ffmpeg label is only added to “Configure” step for the job that has it enabled (GeneralsMD win32-vcpkg)

Add it everywhere else (job name and other steps) so that it's clear what build has FFmpeg support enabled

No functional changes

qodo-free-for-open-source-projects · 2026-09-03T11:58:26Z

PR Summary by Qodo

Show FFmpeg support in vcpkg workflow labels

⚙️ Configuration changes 🕐 Less than 5 minutes

AI Description

• Appends +ffmpeg to the toolchain job name when FFmpeg is enabled.
• Identifies FFmpeg-enabled builds in CMake build and artifact upload step labels.

High-Level Assessment

The conditional label suffix matches the workflow's existing tools and extras naming convention and is the most direct approach. Centralizing the generated label was considered but would add unnecessary complexity for three display-only references.

Files changed (1) +3 / -3

Other (1) +3 / -3

build-toolchain.ymlLabel FFmpeg-enabled workflow jobs and steps +3/-3

Label FFmpeg-enabled workflow jobs and steps

• Appends a conditional '+ffmpeg' suffix to the build job, CMake build step, and artifact upload step display names. This improves workflow visibility without changing build execution or uploaded artifact names.

.github/workflows/build-toolchain.yml

qodo-free-for-open-source-projects · 2026-09-03T11:58:46Z

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

1. Collect step omits FFmpeg 🐞 Bug ≡ Correctness

Description

The artifact collection step still omits the +ffmpeg suffix, so the FFmpeg-enabled build remains
indistinguishable at that step despite the PR's labeling goal. This creates inconsistent labels
within the same job.

Code

.github/workflows/build-toolchain.yml[181]

+      - name: Upload ${{ inputs.game }} ${{ inputs.preset }}${{ inputs.tools && '+t' || '' }}${{ inputs.extras && '+e' || '' }}${{ inputs.ffmpeg && '+ffmpeg' || '' }} Artifact

Evidence
The workflow passes ffmpeg: true for the win32-vcpkg build, and the configure, build, and upload
step names include the conditional suffix. The collection step at line 164 uses the same
preset/tools/extras label pattern but is the sole artifact lifecycle step without the FFmpeg suffix.
.github/workflows/ci.yml[149-166]
.github/workflows/build-toolchain.yml[141-164]
.github/workflows/build-toolchain.yml[181-185]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The FFmpeg-enabled workflow labels the configure, build, and upload steps with `+ffmpeg`, but not the artifact collection step.

## Issue Context
Use the same conditional `${{ inputs.ffmpeg && '+ffmpeg' || '' }}` suffix already applied to the surrounding step names.

## Fix Focus Areas
- .github/workflows/build-toolchain.yml[164-164]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Tip of the day

💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

greptile-apps · 2026-09-03T11:59:38Z

Greptile Summary

This PR makes FFmpeg-enabled vcpkg builds easier to identify in GitHub Actions without changing build or artifact behavior.

Appends +ffmpeg to the reusable build job name when FFmpeg is enabled.
Adds the same conditional label to the build, collection, and upload step names.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
.github/workflows/build-toolchain.yml	Adds consistent conditional `+ffmpeg` text to workflow display names while leaving commands and artifact identifiers unchanged.

_{Reviews (2): Last reviewed commit: "Add '+ffmpeg' label to build job and ste..." | Re-trigger Greptile}

xezon · 2026-09-03T12:57:20Z

Do we need this at all in the title? What is it good for?

tsunamistate · 2026-09-03T13:06:50Z

So that CI jobs display the +ffmpeg label when it's enabled
In my previous PR, GeneralsMD win32-vcpkg has FFmpeg support enabled, but it's only shown in one step (Configure)

Now it shows in both the job name and all steps, so it's clear that this has FFmpeg support

Why do you need to know which build has FFmpeg support? For now, support is incomplete, FFmpeg build has no audio is cutscenes, but it's useful to have a build for testing.

xezon · 2026-09-03T13:44:37Z

Do we need this extra keyword in the target name? Can we not imply it with vcpkg, that it always has ffmpeg enabled? Why do we build only one of the vcpkg targets with ffmpeg? Should we not build all of them with it?

tsunamistate · 2026-09-03T14:14:46Z

Can we not imply it with vcpkg, that it always has ffmpeg enabled?

I just made FFmpeg optional and disabled by default in 3184, so vcpkg does NOT imply enabled FFmpeg support

Why do we build only one of the vcpkg targets with ffmpeg?

@bobtista sorry to bother you, but can you please explain why you asked to enable FFmpeg on only one job?
I would assume it's because, a) FFmpeg support is incomplete and b) FFmpeg compilation takes a long time without caching.

tsunamistate · 2026-09-09T09:47:24Z

It shouldn't take this long to merge a missing variable

I believe I have explained everything. If you think this change is not worth merging, close it

xezon

It shouldn't take this long to merge a missing variable

Why?

xezon · 2026-09-09T11:21:30Z

          cmake --preset ${{ inputs.preset }} $buildFlags

-      - name: Build ${{ inputs.game }} with CMake Using ${{ inputs.preset }}${{ inputs.tools && '+t' || '' }}${{ inputs.extras && '+e' || '' }} Preset
+      - name: Build ${{ inputs.game }} with CMake Using ${{ inputs.preset }}${{ inputs.tools && '+t' || '' }}${{ inputs.extras && '+e' || '' }}${{ inputs.ffmpeg && '+ffmpeg' || '' }} Preset


Instead of duplicating this mega long string concat, can we do it once higher up and then cache it and reuse it under a new name? That then also carries less risk of breaking again in the future when adding another new substring to it.

I don't know how to do that

Chat Gippy says:

Yes. In GitHub Actions, you can concatenate strings and store the result in a local variable, but the exact syntax depends on where you want the variable to exist.

Within a shell step

This is usually the simplest:

- name: Build string run: | PREFIX="hello" SUFFIX="world" RESULT="${PREFIX}-${SUFFIX}" echo "$RESULT"

Persist it for later steps

If you want the variable available in subsequent steps, write it to $GITHUB_ENV:

- name: Build string run: | PREFIX="hello" SUFFIX="world" echo "RESULT=${PREFIX}-${SUFFIX}" >> "$GITHUB_ENV" - name: Use string run: | echo "$RESULT"

Using GitHub Actions expressions

You can also concatenate values directly in expressions using format():

env: RESULT: ${{ format('{0}-{1}', 'hello', 'world') }}

Or, for example:

env: IMAGE_TAG: ${{ format('{0}:{1}', github.repository, github.sha) }}

One important distinction: GitHub Actions expressions don't have general-purpose local variable assignment like let x = .... For temporary variables, use the shell; for values needed by later steps, use $GITHUB_ENV; for job/workflow configuration, use env:.

If you show me the specific string you're trying to construct, I can give you the cleanest syntax for your workflow.

qodo-free-for-open-source-projects Bot reviewed Sep 3, 2026

View reviewed changes

Comment thread .github/workflows/build-toolchain.yml

Add '+ffmpeg' label to build job and steps

2d27959

tsunamistate force-pushed the add-ffmpeg-label branch from 61cfcbd to 2d27959 Compare September 3, 2026 12:00

tsunamistate changed the title ~~build(vcpkg): Add missing '+ffmpeg' label to build job and steps~~ build(ci): Add missing '+ffmpeg' label to build job and steps Sep 3, 2026

xezon reviewed Sep 9, 2026

View reviewed changes

Uh oh!

Conversation

tsunamistate commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 3, 2026

PR Summary by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot commented Sep 3, 2026

Code Review by Qodo

Uh oh!

Uh oh!

greptile-apps Bot commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

xezon commented Sep 3, 2026

Uh oh!

tsunamistate commented Sep 3, 2026

Uh oh!

xezon commented Sep 3, 2026

Uh oh!

tsunamistate commented Sep 3, 2026

Uh oh!

tsunamistate commented Sep 9, 2026

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

xezon Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

tsunamistate Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

xezon Sep 10, 2026

Choose a reason for hiding this comment

Within a shell step

Persist it for later steps

Using GitHub Actions expressions

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tsunamistate commented Sep 3, 2026 •

edited

Loading

greptile-apps Bot commented Sep 3, 2026 •

edited

Loading