Skip to content

refactor(cli/self-update): move process out of InstallOpts - #4998

Merged
ChrisDenton merged 2 commits into
rust-lang:mainfrom
rami3l:refactor/more-installopts
Aug 7, 2026
Merged

refactor(cli/self-update): move process out of InstallOpts#4998
ChrisDenton merged 2 commits into
rust-lang:mainfrom
rami3l:refactor/more-installopts

Conversation

@rami3l

@rami3l rami3l commented Aug 6, 2026

Copy link
Copy Markdown
Member

Follow-up of #4996.

Initially I thought moving process into InstallOpts is more convenient since many method calls rely on this value. However since certain operations also rely on Cfg which also contains process, this might cause confusions regarding which process to use. In our particular case, they are fortunately always the same value.

With this change, process is systematically injected when calling methods of InstallOpts, whether directly or through Cfg.

This way the `process` is always injected when calling methods of
`InstallOpts`, whether directly or through `Cfg`.
@rami3l
rami3l requested a review from ChrisDenton August 6, 2026 14:47
@rami3l
rami3l force-pushed the refactor/more-installopts branch from 54634d1 to 564ba7c Compare August 6, 2026 14:51
@rami3l
rami3l marked this pull request as ready for review August 6, 2026 14:53

@ChrisDenton ChrisDenton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I admit looking at the diff I don't feel too strongly about this but it makes sense on a conceptual level.

View changes since this review

@ChrisDenton
ChrisDenton added this pull request to the merge queue Aug 7, 2026
Merged via the queue into rust-lang:main with commit 79766cf Aug 7, 2026
90 of 93 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