Skip to content

Conversation

@geofflamrock
Copy link
Owner

Using merge is a safe default update strategy, however might be a bit unexpected if someone uses the tool for the first time and doesn't really know this is going to happen, or has configured on a per-repo basis and forgets to do so for a new repo. There currently also isn't any logging to indicate which strategy is used, it just happens.

This PR changes the default to ask the user instead of using merge. It also provides examples of how to configure the setting for the future.

@geofflamrock geofflamrock added enhancement New feature or request minor Increment the minor version when merged and removed minor Increment the minor version when merged labels Jun 30, 2025
@geofflamrock geofflamrock force-pushed the select-update-strategy branch from 7b32bb0 to aa18b99 Compare June 30, 2025 09:02
@geofflamrock geofflamrock requested a review from Copilot June 30, 2025 09:03
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR changes the default branch update behavior to prompt the user instead of always merging, surfaces the chosen strategy in logs with configuration hints, and adds tests covering scenarios where no strategy is preconfigured.

  • Prompt for update strategy when not specified or configured, with log messages and git-config examples.
  • Refactor UpdateStack to return the chosen UpdateStrategy and update callers to use its result.
  • Extend test suites to cover merge/rebase flows when no config exists and update README accordingly.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/Stack/Commands/Remote/SyncStackCommand.cs Capture returned strategy and compute forceWithLease solely from it
src/Stack/Commands/Helpers/StackHelpers.cs Change UpdateStack to return UpdateStrategy, prompt user if unset, and log examples
src/Stack/Commands/Helpers/Questions.cs Add SelectUpdateStrategy prompt constant
src/Stack.Tests/Commands/Stack/UpdateStackCommandHandlerTests.cs Add tests for rebase/merge when no config exists
src/Stack.Tests/Commands/Remote/SyncStackCommandHandlerTests.cs Add tests for prompting merge/rebase in remote sync when no config
README.md Update default strategy description to reflect interactive prompt
Comments suppressed due to low confidence (3)

src/Stack/Commands/Helpers/StackHelpers.cs:501

  • The signature of UpdateStack changed from void to returning UpdateStrategy. Please verify all other callers (outside of these two consumers) have been updated to handle the return value to avoid breaking callers.
    public static UpdateStrategy UpdateStack(

src/Stack.Tests/Commands/Stack/UpdateStackCommandHandlerTests.cs:668

  • In the merge scenario test, stub inputProvider.Select(Questions.SelectUpdateStrategy, ...) to explicitly return UpdateStrategy.Merge so the test does not rely on the default enum value and remains robust.
        // Act

README.md:110

  • [nitpick] Consider expanding this section to document that the tool logs the selected strategy and shows git config stack.update.strategy examples, so users know how to persist their choice.
You will be asked to select an update strategy if none is supplied or configured.

@geofflamrock geofflamrock force-pushed the select-update-strategy branch from aa18b99 to a3387de Compare June 30, 2025 09:05
@geofflamrock geofflamrock added this to the 0.13.0 milestone Jun 30, 2025
@geofflamrock geofflamrock merged commit 3f9a37d into main Jun 30, 2025
16 checks passed
@geofflamrock geofflamrock deleted the select-update-strategy branch June 30, 2025 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants