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
Copy link
Copy Markdown
Member

@lbussell lbussell commented May 2, 2025

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
Copy link
Copy Markdown
Member

mthalman commented May 5, 2025

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
Copy link
Copy Markdown
Member Author

lbussell commented May 5, 2025

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
102 checks passed
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