From Class: Branches or Forks

[Jooms]

There were two options discussed in class, let me try to explain both, the pros and cons, and which one is the easiest.

Overview

Branches (Easiest) - Recommended

Develop directly on the repo.

Steps

1. Cloning the team's repo directly
2. Create a branch and work on it
3. Push your branch to the team's remote (github)
4. Create a pull request from your branch to the main branch (both on the team's remote)
5. Delete your branch after your PR is merged

Forks

Develop on your own copy of the repo, and request the changes get added.

Steps

1. Fork the repo to your own account
2. Create a branch and work on it
3. Push your branch to your remote repo (github)
4. Create a pull request from your branch on your remote to the main branch on the team's remote
5. Delete the branch on your remote after your PR is merged

This involves creating a cloning the repo directly, creating a local branch, pushing it to the remote (github), requesting a pull requests, and deleting the branch after your PR is merged.

Pros and Cons

Branches

Pros:

• You can easily send someone your branch, and they will be able to check it out locally

Cons:

• Anyone with permissions to create branches in the team repo can modify your (remote) branch

Forks

Pros:

• You're the only one that can modify your branch, unless you explicitly allow others to.

Cons:

• It's difficult for your team members to checkout your branch, they'll either need to check out your repo first or handle some crazy git configurations to do so.

[AbhinavReddy-Dev]

@Jooms Thanks for putting it together!

[hardikpatil]

Agreed

[kbuffardi]

A related point that was brought up:

If we're branching (and not forking), then what naming convention(s) would you like to use so it is easy for contributors to identify which branch(es) correspond with which issue(s)?

I am open to what the consensus of the class prefers. All I ask is that once a convention is agreed upon, we update Contributing.md to document it for consistency.

[Jooms]

If we're branching (and not forking), then what naming convention(s) would you like to use so it is easy for contributors to identify which branch(es) correspond with which issue(s)?

I actually disagree that branch naming is important enough to warrant a convention that we apply repo-wide.

I expect two types in our future (and please correct me if I'm missing some)

1. A branch for a single dev's change
2. A feature branch for multiple devs to work on.

In 1), I don't think it matters what the branch name is. It will only come up in the Pull Request, and there's a helpful "copy" button next to the branch name, so reviewers that run it locally don't have to worry about typing it out.

In 2), I think the branch name only matters to the group of devs working on it. They should spend as little time as possible coming up with what label they want to use, and then work on that. They can make it as expressive or short-to-write as they want, and it doesn't really affect anyone else.

[kbuffardi]

I tend to agree that branch names mostly only really matter to the developers working on the branch. It's useful for that name to at least be descriptive, though. From the standpoint as the repo owner, I prefer people to delete branches once a PR has been accepted/merged... but in case anyone forgets to do so, it helps if I can read the branch name and get a good idea of what its purpose was (and delete old/stale branches when evident it is not something being worked on). The travis-ci branch is a good example of this.

One argument for a more structured naming convention is if we wanted to apply different protection rules for different types of issues they address, naming conventions can be set up to apply the corresponding protection policies. However, for the time being, I don't see a need for any protection for branches besides main so that argument may be moot.

[Jooms]

but in case anyone forgets to do so, it helps if I can read the branch name and get a good idea of what its purpose was (and delete old/stale branches when evident it is not something being worked on). The travis-ci branch is a good example of this.

Yea. I can see that.
Though, I think when we get to the "This branch has been here for awhile and I want to figure out if we should keep it or not" is a problem that could be solved in a better way. Perhaps there's an argument that if a branch has been open for over X weeks without a new commit or attached PullRequest should be cleaned up? With that travis-ci branch or any of the other old branches, did we pick them back up for development or even use them for notes?

if we wanted to apply different protection rules for different types of issues they address

I like these. I think they'd work well with "release" branches (as is called out in docs and demos), though I don't think we're ready for release branches yet.

[hardikpatil]

Say 5 people working on feature branch X of issue A, and 5 of each working on different modules of X, so the individual will separate branch (feature-X-P, feature-X-Q, feature-X-R...) checkout from X and later will be merged with X(after PR review and approval)

@Jooms @kbuffardi whatcha thoughts?

[hardikpatil]

one of the 5 or 2 will be requested for PR review and if all good will be merged with X and will be created PR to merge for main(that would be the final PR review to @kbuffardi )

[kbuffardi]

Both reviews should be done by students. Unless absolutely necessary to get my input, I would like to give the class the authority to review and accept PR's so that you all can experience the responsibilities associated with that decision.

[Jooms]

@hardikpatil

IMO, and this is completely my opinion on this one, but I have seen that setup before (one longer-lived feature branch that multiple devs merged into) and it had a few downsides.

1. Reviews merging into the feature branch weren't taken seriously because:
   1. they were trying things out or it wasn't complete enough to review.
   2. there was no CI setup on those branches to require linting / building / etc.
2. When the feature branch was "ready" to merge into main, the PR was huge, so no one could actually review it well. They either just ran it locally and clicked around a little, or flat-out stamped it.
   1. The combination of this and the bit above means that buggy / ugly / unreviewed code just makes it into the repo without review.
3. While the feature branch was being worked on, there are changes landing in the main branch.
   1. Which means the feature branch has to keep accepting merge commits / rebasing
   2. and then the dev branches on the feature branch have to also accept merge commits / rebase.

As an alternative idea. I'd recommend the developers who are working on a feature, focus on getting bite-sized pieces contributed to the main branch. Then every review is small and can be reviewed in detail, all of the CI is applied at every step, and it works well with all the tooling GitHub already provides. This also means that every small piece handles rebases / merge commits as needed, but they aren't alive long enough to worry about doing that too many times or drifting from the main codebase.

The tough part then becomes breaking down the work into bite-sized and parallelizable pieces.

[AbhinavReddy-Dev]

So, have we finalised on the naming convention for the branches?

[Jooms]

Current status: I think there's discussion going on in the comments above. We should leave this open for at least a week to allow people to have time to read and discuss before we decide on anything.

[kbuffardi]

Given the discussions in this thread, I have proposed some updates to the guidelines for contributing in PR #58

I plan to reserve some time in class on March 21, 2023 to introduce/discuss this so I don't expect it to be accepted before then. However, if anyone has input on the policies themselves, this thread is appropriate for discussion. If there are fixes/critiques to the PR itself, you can comment in that PR.