Skip to content

feat: add --branch option to clone or checkout different dotfiles branch #8331

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jul 6, 2023

Conversation

Emyrk
Copy link
Member

@Emyrk Emyrk commented Jul 5, 2023

If --branch is specified, the clone will clone that branch.

The code will always do a git branch --show-current and checkout the specified branch if the flag is provided.

Fixes: #8282

How it works

  1. If dotfiles does not exist
    • If --branch is specified, git clone uses git clone --branch <xxx> ...
  2. If dotfiles does exist
    • Runs git pull (
      • Always run this before a checkout because the new branch might not be fetched. So a git checkout might fail before a pull.
  3. If --branch exists
    • Run git checkout <branch>
    • Run git pull --ff-only (The earlier pull was on the wrong branch)

@Emyrk Emyrk changed the title feat: --branch option to clone/checkout different dotfiles branch feat: add --branch option to clone or checkout different dotfiles branch Jul 5, 2023
@Emyrk Emyrk requested a review from mafredri July 6, 2023 13:43
Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

Nice feature! Might've misunderstood some parts but left a few questions.

@Emyrk Emyrk requested a review from mafredri July 6, 2023 17:58
Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

Once the stdout thing is fixed, I see no blockers. LGTM and thanks for answering my questions! 👍🏻

if err != nil {
// Do not block on this error, just log it and continue
_, _ = fmt.Fprintln(inv.Stdout,
cliui.DefaultStyles.Error.Render(fmt.Sprintf("Failed to use branch %q (%s), continuing...", err.Error(), gitbranch)))
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to use a non-zero exit code if this happens? Otherwise the user might expect one thing but receive another due to the silent failure. (Not saying we can't continue from here, but set a branchFailed = true and then exit 1 at the end.

Copy link
Member Author

Choose a reason for hiding this comment

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

That is an idea. I am not sure, I was copying the existing behavior.

See this:

if !dotfilesExists {

If the git pull --ff-only fails then we continue, even though we are now operating in a stale dotfiles. I was maintaining that behavior of continuing.

If we decide to exit code 1, we should go back and also change the previous behavior.

@Emyrk Emyrk enabled auto-merge (squash) July 6, 2023 20:13
@Emyrk Emyrk merged commit 9f5bc7c into main Jul 6, 2023
@Emyrk Emyrk deleted the stevenmasley/dotfiles_options branch July 6, 2023 20:24
@github-actions github-actions bot locked and limited conversation to collaborators Jul 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow passing git arguments to coder dotfiles
2 participants