Skip to content

fix: unexport AddInitrdRecord and clean up error formatting#555

Open
hemang1404 wants to merge 2 commits intourunc-dev:mainfrom
hemang1404:fix-add-initrd-record
Open

fix: unexport AddInitrdRecord and clean up error formatting#555
hemang1404 wants to merge 2 commits intourunc-dev:mainfrom
hemang1404:fix-add-initrd-record

Conversation

@hemang1404
Copy link
Copy Markdown
Contributor

@hemang1404 hemang1404 commented Apr 6, 2026

Related issues

Description

Unexported the internal helper function AddInitrdRecord by renaming it to addInitrdRecord. Cleaned up the error strings in the file by lowercasing them and using %w standard error wrapping formats.

How was this tested?

Verified that the package successfully compiles locally by compiling against the target Linux architecture ($env:GOOS="linux"; go build ./pkg/unikontainers/initrd/...).

LLM usage

Used chatgpt to verify changes

Checklist

  • I have read the contribution guide.
  • The linter passes locally (make lint).
  • The e2e tests of at least one tool pass locally (make test_ctr, make test_nerdctl, make test_docker, make test_crictl).
  • If LLMs were used: I have read the llm policy.

Fixes: urunc-dev#502
Signed-off-by: hemang1404 <hemangsharrma@gmail.com>
@netlify
Copy link
Copy Markdown

netlify bot commented Apr 6, 2026

Deploy Preview for urunc canceled.

Name Link
🔨 Latest commit bbd8ca2
🔍 Latest deploy log https://app.netlify.com/projects/urunc/deploys/69d4e84f3b077a0008063046

@cmainas
Copy link
Copy Markdown
Contributor

cmainas commented Apr 7, 2026

Thank you @hemang1404 for this PR.

Is there any particular reason for not running the tests, or at least the linter?

@hemang1404
Copy link
Copy Markdown
Contributor Author

Hi @cmainas, I ran the unit tests locally and all the checks passed but I noticed that make lint was failing on my branch due to a pre-existing indentation issue in a different file (pkg/unikontainers/hypervisors/qemu.go.
lint error

After fixing the indentation issue there, make lint passes successfully.
lint

Signed-off-by: hemang1404 <hemangsharrma@gmail.com>
@hemang1404 hemang1404 force-pushed the fix-add-initrd-record branch from 5f1ccf8 to bbd8ca2 Compare April 7, 2026 11:19
@cmainas
Copy link
Copy Markdown
Contributor

cmainas commented Apr 9, 2026

I see, we need to check why it fails locally and not in our CI. The linter warning is valid though.

@IrvingMg
Copy link
Copy Markdown
Contributor

I see, we need to check why it fails locally and not in our CI. The linter warning is valid though.

I looked into the issue, and it seems CI doesn’t report it because the only-new-issues flag is enabled in the workflow:

only-new-issues: true

As I understand it, this flag filters issues based on the diff. Since there are no new changes touching the problematic code, the issue isn’t reported in CI.

Locally, however, the make lint target doesn’t use this filter, which is why the issue shows up.

Yet, using only-new-issues seems to be the standard approach, so my suggestion would be to just fix the formatting issue and keep only-new-issues as is.

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.

Unexport internal function AddInitrdRecord

3 participants