KhronosGroup/Vulkan-LoaderAndValidationLayers

Too many branches

nsubtil opened this issue · 13 comments

There are over 200 different branches in this repository, which makes it hard to understand which branches are active.

From a quick look, it seems like most branches simply contain a bug fix or something like that and can probably be removed. Can we get this cleaned up?

We went through a purge as recently as March (#2450), but there is likely to be at least some new cruft that can go.

Currently 109 branches. I just got myself down to 10. @jzulauf-lunarg and @daveh-lunarg look to have a handful each that can be deleted.

@mark-lunarg Well there is also old cruft too. Those 2 year old, 1000s of commits behind branches. Unfortunatelly they are not tied to PRs, so it is hard to evaluate if they were ever merged, and ready to be deleted....

@nsubtil note that all work takes place on master branch, and the only other branches of general interest are typically the sdk branches that correspond to SDK releases. All other branches can be ignored unless you know you have a specific interest in one.

We'll get some more branches cleaned up to close out this issue, but given the number of developers and activity in the repo I'm not surprised if 100+ branches exist at any time.

Thanks, that's a sizable improvement.

Still, having 100+ branches means it's hard to find the useful branches. Ideally we would have only active branches in the repo.

Is there a reason we can't do fork-and-pull? That tends to solve the problem quite nicely, plus it removes the need to prune dozens of branches every now and then.

@nsubtil Thinking the same way :) That was answered in #2450.

#2450 indicates some internal CI that can't cope with pull requests --- could we address that? Most common CI solutions seem to integrate well with GitHub and can cope with pull requests just fine.

Currently, there are at least 16 developers actively working in this repo, so that's around 5 branches per developer not counting the sdk/master branches. I think @cnorthrop was trying to say that it is common to use Github CI to test branches, which is normal workflow even outside of PRs.

cdwfs commented

I just deleted a handful of my older branches as well. Down to 91!

@cnorthrop can you clarify the statements around CI being a blocker for push-pull? Is the current model by design, or is it a side-effect of a technical issue around CI?

Our internal CI supports testing branches for pre-commit build/test, but only from this repo, not downstream user repos. It is not tied to the Github pull request process. We only actively track master otherwise.

Another factor:

If "internal CI" is changed to be able to process GitHub pull requests from downstream repos (in order to make upstream branches unnecessary), then that would require the internal CI status to be reflected back up into the GitHub Pull Request "Completed Checks" panel. Like the existing Travis-CI and AppVeyor checks in this panel, there would need to links back to the internal CI machines' logs so that people can click through to see what went wrong. This would mean that the internal CI machine would need to be exposed to the outside network, which requires more effort and expense to manage and deal with security, etc.

It's been about 10 days and we're still at about 90 branches.

I collected the "owners" of the branches with:

git for-each-ref --format='%(authordate:format:%m/%d/%Y %I:%M %p)    %(align:30,left)%(authorname)%(end)%(align:30,left)%(authoremail)%(end) %(refname:strip=3)' --sort=authordate refs/remotes | sort -k 7 | grep -v "sdk-1"

I scraped the email addresses using this:

git for-each-ref --format='%(authordate:format:%m/%d/%Y %I:%M %p)    %(align:30,left)%(authorname)%(end)%(align:30,left)%(authoremail)%(end) %(refname:strip=3)' --sort=authordate refs/remotes | sort -k 7 | grep -v "sdk-1" | cut -f 2 -d '<' | cut -f 1 -d '>' | sort | uniq

And then I sent them all an email with the results of the first command attached, asking them to clean up any unneeded branches.

I think that this is going to be the best we can do for now.

There is a lot of friction with going the extra step of banning local branches completely, mainly in the area of adapting extant CI systems, as explained above. In addition, there is issue #2533, which may make any additional branch cleanup efforts a bit moot.

@nsubtil, I'm going to close this now. But feel free to reopen if you think that there is any value in additional effort here.