CCNYRoboticsLab / phidgets_drivers

phidgets_drivers
6 stars 15 forks source link

Hydro - latest changes #10

Closed muhrix closed 10 years ago

muhrix commented 10 years ago

Just keeping branch master up to date with the branch hydro, since master is the default branch.

mintar commented 10 years ago

Hmm, you shouldn't have done it this way. Now, the history of master and hydro is different, although it should be the same. It's going to be hard from now on to keep the two in sync. What you should have done is:

git checkout master
git merge hydro
git push

That way, the two branches would point to the same commit, and it would be immediately obvious that they are aliases. Now, there's nothing we can do about it; we could do a push -f, but since we're working on public history, that's a no-go. :-(

mintar commented 10 years ago

Maybe next time just leave the pull request open for a day or so, so others can give you feedback before merging.

muhrix commented 10 years ago

It appears we have different approaches regarding the best practices in managing git repos. Considering I'm relatively new to git, I'm assuming I'm not right and I do apologise for that.

My intention (and good job I didn't do it) was to merge hydro into master and delete the branch hydro (there's even a tempting button here which I almost clicked when I merged).

The rationale is that this catkin package works with both Groovy and Hydro (I tested) and I thought there was no point in keeping branches groovy and hydro, which are currently not in sync. fuerte is rosbuild-based, so it's a different matter.

Not sure if it makes any sense now that I explained my rationale, or if it still is conceptually wrong... In any case, I think we should have master for the current release, fuerte for the sake of keeping the original, rosbuild-based implementation (not maintained anymore) and temporary branches for bug fixes, updates and things alike, but always merging into master, which works with Groovy and Hydro.

mintar commented 10 years ago

No need to apologize! :-)

Personally, I prefer to have one branch per ROS distro vs. a common branch (master) for several distros. Advantages/disadvantages of the multi-branch approach:

(-) Overhead of keeping multiple branches in sync (+) future-proof: you say that the current hydro branch also works in groovy. But what if we have to introduce a breaking change in the future, so that it only works in hydro? We would have to fork off a new groovy branch, and all existing users would have to switch; but we have no way of notifying them. (+) You don't have to test on all distros before committing something. You can just commit something to hydro, then later test whether it also works in groovy. If yes, you merge it into the groovy branch. (+) It's immediately obvious which ROS versions are supported by which branch.

So with the current setup, what you should do is:

git checkout groovy
git merge hydro
gitk --all  # (to check that groovy and hydro indeed point to the same commit)
git push

One other point: Of course, the master branch is redundant, and maybe we should remove it (after setting the default branch to hydro). On the other hand, there might be people who are using it, so I'm a bit reluctant to do that.

muhrix commented 10 years ago

We'll have to keep master, as I do not have privileges to change the default branch of this repo. I reckon that, as long as we keep master in sync with the latest ROS distro branch, it's fine to have it.

hydro is now merged into groovy.