Cleanup and refactor of versioncontrol_git

The log parser is one of those things that should ultimately be based on our gitlib/cligit implementation...although I know from doing this with versioncontrol_svn that integration isn't SO tight that it can't be pretty easily refactored in later. However, it does seem like more work to need to refactor later...I'd kinda rather see cligit done first?

Fair enough, we don't need it for phase 1 - not for this, or anything else. And that it's nearly done is the most important thing anyway.

Yup, I was just looking this morning to see if we had any post-receive hooks. Don't think we do, and that's definitely the next step.

I just notice Druplicon message on IRC, we need auto-lists for notify isssues to maintainers :-)

Well, yep, I review the changes, and I just re-rolled the patch against the DRUPAL-6--2 branch to make it easy to try.

Please note that, since there was the vcs api bug #714198: VersioncontrolRepository::getLabels is broken, you need the last dev tarball to try it.

Maybe a sdboyer try before the commit? :-)

marking as need review again to avoid forget re-reading it :-)

need this for phase 2 of the migration

yeah, it's on me :) i'll get to it ASAP

This is _awesome_. So glad I went through this. I've incorporated it locally into my changes, so marking it fixed, though I can't commit back to CVS for the next couple days until the corresponding entity changes in the main versioncontrol branch get stable, too.

Nevermind, just gonna put this in straightaway since it applies well to the pre-entities work.

@CorniI Now that I've finally gone through most of all this in considerable detail, I finally figured out what's been bothering me about it. I found the way you broke up some of the logic in the log parser to be counterintuitive. I liked that there was a clean, easily-readable top-level function (versioncontrol_git_log_update_repository()) that called lots of secondary functions (e.g., versioncontrol_git_log_parse_and_insert_commit()). But there was another level beyond that second one, which is where it started to get irksome. I don't mind deep nesting in general - the problem I was having here was that most of these functions contain important logical components, like invoking the git binary to retrieve some data. Removing that logic from the main flow of code forced me to switch between the main function and subfunction repeatedly, which made for a difficult grokking experience. So again, while I'm all for breaking up code into logical segments, moving data retrieval too far away from data manipulation can lead to confusion.