- August 29, 2009
- 1,730
- 1,124
- Home Country
-
Netherlands
Hi,
First I'd like to say that I'm happy that you've moved to Git (and GitHub) for your source control. It's a lot better version control then SVN in my opinion, but like you probably experienced, all begin is hard. I've been using Git for a few years now, and looking at the current MediaPortal repository there are a few things that stand out. Note that I'm not a team member nor do very active development on MediaPortal itself, and that Git supports multiple workflows, so this is really just my opinion, but I think it might be useful to you.
If I look at the current repository I see that branches are used a lot - that's a good thing, it's the power of Git. However, If I look at the current repository, there are 119 branches. The branch viewer shows me that more then half of these are already merged into master and Release_1.2.x. I think it's better to delete these stale branches: you don't need them anymore and they are just annoying when navigating through the branch list or history.
The base for branches also doesn't seem to be always correct. For example the topic branches for bugs 3772 and 3781 seem to be based on master, but are merged into Release_1.2.x later on. That doesn't give much problems now (with master and Release_1.2.x being very close), but when feature branches are merged into master it is going to give problems: with the merge of the topic branch (the BUG-XXXX branch), you'll also pull in the new features into Release_1.2.x.
Also, I don't really get how you are handling pull requests like this one. The creation of a new branch isn't necessary at all: you can easily perform cross-repository merges with Git (that's what most big open source projects do on a daily basis). If I then take a look at the branch, I see that there's a new commit created with exactly the same patch as from the pull request (losing author information). I think you are missing an important feature of Git that helps avoiding work like this here. You can, after discussion, directly merge DieBagger's branch into your own repository. When everbody is using Git applying patches manually shouldn't be needed anymore (that was actually the reason why Linus Torvalds created Git: manual patching was too slow and error-prone).
First I'd like to say that I'm happy that you've moved to Git (and GitHub) for your source control. It's a lot better version control then SVN in my opinion, but like you probably experienced, all begin is hard. I've been using Git for a few years now, and looking at the current MediaPortal repository there are a few things that stand out. Note that I'm not a team member nor do very active development on MediaPortal itself, and that Git supports multiple workflows, so this is really just my opinion, but I think it might be useful to you.
If I look at the current repository I see that branches are used a lot - that's a good thing, it's the power of Git. However, If I look at the current repository, there are 119 branches. The branch viewer shows me that more then half of these are already merged into master and Release_1.2.x. I think it's better to delete these stale branches: you don't need them anymore and they are just annoying when navigating through the branch list or history.
The base for branches also doesn't seem to be always correct. For example the topic branches for bugs 3772 and 3781 seem to be based on master, but are merged into Release_1.2.x later on. That doesn't give much problems now (with master and Release_1.2.x being very close), but when feature branches are merged into master it is going to give problems: with the merge of the topic branch (the BUG-XXXX branch), you'll also pull in the new features into Release_1.2.x.
Also, I don't really get how you are handling pull requests like this one. The creation of a new branch isn't necessary at all: you can easily perform cross-repository merges with Git (that's what most big open source projects do on a daily basis). If I then take a look at the branch, I see that there's a new commit created with exactly the same patch as from the pull request (losing author information). I think you are missing an important feature of Git that helps avoiding work like this here. You can, after discussion, directly merge DieBagger's branch into your own repository. When everbody is using Git applying patches manually shouldn't be needed anymore (that was actually the reason why Linus Torvalds created Git: manual patching was too slow and error-prone).
)). The reason why they are still there is that the fixes weren't merged to the master at the same time when they were merged to the 1.2.x branch - there was huge bulk of extra work for gathering and handling all the SVN based error fixes after GIT migration was done. Currently we are lacking a bit on the developers - everyone has their hands full of things to do (mostly in the non-MP areas).