Skip to content

refactor: perfect-forward remaining error ctors - #16309

Merged
xokdvium merged 1 commit into
NixOS:masterfrom
roberth:error-arg-forward
Aug 16, 2026
Merged

refactor: perfect-forward remaining error ctors#16309
xokdvium merged 1 commit into
NixOS:masterfrom
roberth:error-arg-forward

Conversation

@roberth

@roberth roberth commented Aug 16, 2026

Copy link
Copy Markdown
Member

Follow-up on #16296 (comment) that I had already started.

Uneventful, except FileTransferError used args twice: once for the base class, once for a second HintFmt that overwrote the first result. Can't forward twice, so now it formats once and mutates in place.

Assisted-by: Claude Code (Claude Opus 5)

Motivation

cleanup

Context


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

Uneventful, except `FileTransferError` used `args` twice: once for the base
class, once for a second `HintFmt` that overwrote the first result. Can't
forward twice, so now it formats once and mutates in place.

Assisted-by: Claude Code (Claude Opus 5)
Comment thread src/libutil/archive.cc
static SerialisationError badArchive(std::string_view s, Args &&... args)
{
return SerialisationError("bad archive: " + s, args...);
return SerialisationError("bad archive: " + s, std::forward<Args>(args)...);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems pretty cursed that we dynamically construct the format string here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

But that's pre-existing, so meh

@xokdvium xokdvium left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Probably inconsequential, but I like the consistency and good practices so LGTM.

@xokdvium
xokdvium added this pull request to the merge queue Aug 16, 2026
Merged via the queue into NixOS:master with commit e3214ea Aug 16, 2026
16 checks passed
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.

2 participants