Skip to content

Conversation

@BillyONeal
Copy link
Member

@BillyONeal BillyONeal commented Jan 13, 2026

I did this mostly to see if copilot-cli could do it, and it was able to do most of it. There's no 'business case' to merge it and it will create merge conflicts. But it is a style change I've wanted to do for ages and just never found the time to do; the changes seemed 'local' enough that LLMs may be able to do it so that's what I tried. Would be totally happy to throw this away as a result.

Note that src/vcpkg/commands.check-support.cpp was previously copying the PlatformExpression::Context when the behavior everywhere else in vcpkg is to use the returned context directly, so that is a behavior change.

Remaining Optional<T&> uses:

  • Tests for Optional<T&> itself
  • Tests for adapt_context_to_expected's handling of Optional<T&> (in particular, that that needs to be treated like an lvalue even though a returned Optional<T&> is usually a prvalue)
  • InstallPlanAction::source_control_file_and_location removed in Remove optionality in ActionPlan and Summary #1894

@BillyONeal BillyONeal changed the title Replace most Optional<T&> with T*. Replace most Optional<T&> with T*. Jan 13, 2026
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.

1 participant