Skip to content
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

tap-new: add option for branch name #8932

Merged
merged 2 commits into from Oct 16, 2020

Conversation

Rylan12
Copy link
Member

@Rylan12 Rylan12 commented Oct 15, 2020

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew tests with your changes locally?
  • Have you successfully run brew man locally and committed any changes?

Allow tap-new to accept a --branch flag to specify a branch name for the new tap. Additionally, modify the CI workflows to be compatible with a custom branch name (another CI issue where the label name was missing quotes is fixed as well).

Note: I've opted to make main be the default branch name here instead of master. GitHub repositories are now created with a default branch name of main so I think it's fitting that we create new taps with a branch called main by default. This also makes uploading the new tap repository to GitHub much easier (you can just copy and paste the necessary lines if you create a new repo from the web UI) and will allow all CI to work out of the box without needing to worry about branch names changing. I think it's a good idea to start using the branch name main where we can. The rest of the Homebrew project still uses master as the default branch name, though, so I won't fight hard for this if keeping with master is desired.

Copy link
Member

@issyl0 issyl0 left a comment

Choose a reason for hiding this comment

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

Works for me. Matching the default GitHub behaviour for new repo branch names makes sense. Are there any docs we need to edit that are Homebrew-specific that need to take account of this change to specify --branch=master?

@@ -142,6 +153,7 @@ def tap_new
safe_system "git", "init"
safe_system "git", "add", "--all"
safe_system "git", "commit", "-m", "Create #{tap} tap"
safe_system "git", "branch", "-m", branch
Copy link
Member

Choose a reason for hiding this comment

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

Maybe better specify -b flag for git init directly?

Copy link
Member Author

Choose a reason for hiding this comment

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

I had it this way initially but tests failed. I think the -b flag is a new feature that isn't available in the system versions of git

@Rylan12
Copy link
Member Author

Rylan12 commented Oct 15, 2020

Are there any docs we need to edit that are Homebrew-specific that need to take account of this change to specify --branch=master?

Good question, not sure. I'll take a look

@Rylan12
Copy link
Member Author

Rylan12 commented Oct 15, 2020

@issyl0 I don't think there are any docs that need to be updated. We don't really reference brew tap-new anywhere except here. Think it's worth adding a note there mentioning the default branch name?

@issyl0
Copy link
Member

issyl0 commented Oct 15, 2020

Think it's worth adding a note there mentioning the default branch name?

I think it's fine to just go with the default in that case and not mention it.

@MikeMcQuaid
Copy link
Member

Note: I've opted to make main be the default branch name here instead of master. GitHub repositories are now created with a default branch name of main so I think it's fitting that we create new taps with a branch called main by default. This also makes uploading the new tap repository to GitHub much easier (you can just copy and paste the necessary lines if you create a new repo from the web UI) and will allow all CI to work out of the box without needing to worry about branch names changing.

Makes sense to me 👍🏻

@MikeMcQuaid MikeMcQuaid merged commit 7f422af into Homebrew:master Oct 16, 2020
@MikeMcQuaid
Copy link
Member

Thanks again @Rylan12!

@Rylan12 Rylan12 deleted the tap-new-add-branch-option branch October 16, 2020 13:36
@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Dec 1, 2020
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Dec 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants