Skip to content

Conversation

@ia0
Copy link
Member

@ia0 ia0 commented Jan 23, 2026

This only saves 10 seconds and doesn't really prove that apt-get update is unnecessary since after dpkg --add-architecture i386 we still run it. But at least we run it one time less.

Eventually this should be hermetic, but for now we have floating versions.
@ia0 ia0 added the for:maintainability Improves maintainers life label Jan 23, 2026
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request removes the sudo apt-get update command from the ci-checks workflow. While the goal of reducing unnecessary steps is good, this change could introduce instability into the CI pipeline. The subsequent setup.sh script installs packages using apt, which relies on up-to-date package lists. I've left a comment explaining the potential issue and recommending to keep the apt-get update step for now to ensure build reliability.

I am having trouble creating individual review comments. Click here to see my feedback.

.github/actions/ci-checks/action.yml (16-17)

high

Removing sudo apt-get update here is likely to cause intermittent CI failures. The next step runs ./scripts/setup.sh, which appears to install packages using apt (e.g., ensure apt gcc-multilib and ensure apt libssl-dev:i386 in scripts/setup.sh). Without updating the package lists, apt-get install may fail if the required packages are not in the runner's cache.

Given that the build is not yet fully hermetic and uses floating versions as mentioned in the PR description, it's safer to keep apt-get update to ensure build stability.

@ia0 ia0 merged commit 3837621 into google:main Jan 23, 2026
27 checks passed
@ia0 ia0 deleted the no-update branch January 23, 2026 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

for:maintainability Improves maintainers life

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants