Skip to content

Add dry-run option to update-dependencies tool and pipelines#6414

Merged
lbussell merged 8 commits into
dotnet:nightlyfrom
lbussell:update-deps-dry-run
May 6, 2025
Merged

Add dry-run option to update-dependencies tool and pipelines#6414
lbussell merged 8 commits into
dotnet:nightlyfrom
lbussell:update-deps-dry-run

Conversation

@lbussell

@lbussell lbussell commented May 2, 2025

Copy link
Copy Markdown
Member

Fixes #6389

@lbussell lbussell requested review from a team and jander-msft as code owners May 2, 2025 22:04
Comment thread eng/pipelines/steps/update-dependencies-specific.yml Outdated
Comment thread eng/update-dependencies/CreatePullRequestOptions.cs Outdated
Comment thread eng/pipelines/steps/update-dependencies-specific.yml Outdated
Comment thread eng/pipelines/steps/update-dependencies-specific.yml Outdated
@lbussell lbussell requested a review from jander-msft May 5, 2025 16:46
@mthalman

mthalman commented May 5, 2025

Copy link
Copy Markdown
Member

The descriptions of these options already states that it will skip the PR creation if they are not provided:

new Option<string>("--user") { Description = "GitHub or AzDO user used to make PR (if not specified, a PR will not be created)" },
new Option<string>("--email") { Description = "GitHub or AzDO email used to make PR (if not specified, a PR will not be created)" },
new Option<string>("--password") { Description = "GitHub or AzDO password used to make PR (if not specified, a PR will not be created)" },

So it seems we already have a means to skip the PR creation and don't need a new option. Are those descriptions not true? If not, we should consider providing that functionality anyway since it would make sense when executing the tool from a dev machine where you wouldn't be providing the creds.

@lbussell

lbussell commented May 5, 2025

Copy link
Copy Markdown
Member Author

The descriptions of these options already states that it will skip the PR creation if they are not provided:

new Option<string>("--user") { Description = "GitHub or AzDO user used to make PR (if not specified, a PR will not be created)" },
new Option<string>("--email") { Description = "GitHub or AzDO email used to make PR (if not specified, a PR will not be created)" },
new Option<string>("--password") { Description = "GitHub or AzDO password used to make PR (if not specified, a PR will not be created)" },

So it seems we already have a means to skip the PR creation and don't need a new option. Are those descriptions not true? If not, we should consider providing that functionality anyway since it would make sense when executing the tool from a dev machine where you wouldn't be providing the creds.

See #6414 (comment). Although, after thinking about it some more, the driver behind this change is not to test the code between the credentials short-circuit and the PR creation. So in order to keep things simpler I think yours and Justin's idea makes more sense.

@lbussell lbussell requested a review from mthalman May 6, 2025 14:44
@lbussell lbussell merged commit b029b2b into dotnet:nightly May 6, 2025
lbussell added a commit to lbussell/dotnet-docker that referenced this pull request May 9, 2025
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.

3 participants