Back-link to the commit as a comment on the related issue.

I just started reading through the version control code so I'm not familiar with it yet; I'm just thinking at a higher level to incorporate existing functionality we have at work with Team Foundation Server and Trac.

So as a high level example, say we have a content type ChangesetOperation in the database that contains details of a single commit operation (update/add etc). The ChangesetOperation would have a field called ticket_id, which is extracted from the commit message. E.g. the commit message is:

refs #100

The changeset operation's ticket_id would be set to 100. Then when you view ticket 100, you can see a list of all changesets (basic details to save space). Exactly as you do in ClearQuest/ClearCase and Team Foundation Server.

Hopefully that makes sense. I'm only working at a high abstract level as I'm not familiar enough with version control modules to relate it to their data. I'm trying to find a screenshot to help but I haven't found anything useful yet.

Thanks

Right, that's approximately what I expected. Valid request, although instead of a content type we might just have a table relating two values, the version control operation (a.k.a. vc_op_id) and the issue (a.k.a. nid). Then provide a block that appears in issue nodes and links to the operation URL returned by versioncontrol_get_url_commit_view(). Also provide a setting that the admin can customize for the regular expression in the commit message in order to correctly retrieve the issue id.

This functionality can and should be implemented as an extension module, and is probably pretty easy to implement, using hook_versioncontrol_operation() with the regular expression determining the issue id from $operation['message']. Still takes some work, of course, but all the infrastructure is there and the requirements are straightforward. I'm tagging it with the "Newbie" tag because this one seems like an ideal way to get familar with Version Control API.

new features on the 2.x branch

tagging with views integration

Note: I think we just want a block on issue nodes that shows all commits that reference the issue, not a new comment for every commit that references it. I could be wrong, but it seems like a comment is overkill.

In fact, I'm tempted to call this duplicate with #443000: When viewing an issue, display a list of commits that reference that issue # and move that over here. ;)

Subscribe! :D

I really really really want this:

dww says this is the place to be. :)

Well, I meant they'd be interspersed in the view in chronological order, along with the comments. I'm only so good with firebug. :) They might even be comments from "System message," or the committer, or or whatever.

I think a block showing all commits at the top of the page or something is also useful for those who want an overview of what happened in an issue. But for the purposes of the conversation about a change (commits are a BIG part of that conversation, esp. if/when we move to per-issue repos), having the commits in-context is also super helpful.

I would be happy with either of the above though. I just like to think big. :)

Should probably provide per-project setting for marking issue as fixed on commit and the formatting of the commit/detection of issue reference may have more proper/better ways to be done. I'll leave it up to others to tinker, but this should provide the basic structure.

Since this is being done generically in versioncontrol_project I figured we cannot assume we have uid like we should be able to on d.o thus the if ($operation['uid']).

Also as part of formatting we might want to add label. I just followed webchick's example for this patch.

I love it, for sure

A) Didn't know you could close issue from commit on github, but making a special syntax seems reasonable. Although, I would have to say that per-issue repos, feature branches, and core represent the minority. Defaulting to disabled, but allowing contrib projects to do so since most work on single commit to fix issue wouldn't be a bad idea. I'm fine with making a separate issue since there are many options to discuss. Created #1291886: Support marking an issue as fixed via a commit.

B) Yea noticed the bad comments after uploading. heh

C) Sure, not biggy. Actually went back and forth when writing. My primary concern was that there was a better way to do the comment message maybe even using same formatter used in commitlog?

Sorry for the outdated api file, I just fixed the chunk related to operations so you can take a look at it and the operation class.

Thanks for the effort here!

Just a side note that I'd still like to follow this up with a second issue that introduces a new table that stores associated commits with issue nids, and provides a block or format for adding to issues that lists ALL commit messages that reference the issue number in a single place.

This is looking like a great complement and a great start on a much needed feature though.

I'm also +1 to a mapping table and a block. However, on d.o I'd like to avoid the block rendering in a whole sidebar region, since then we lose tons of screen real estate on the whole issue just for a tiny bit of info at the top of the page. I'd rather that "block" was floated to the right of the "Jump to" links or something. Anyway, that's out of scope for arguing about here, just wanted to mention it while it's on my mind. ;)

D) I just realized this patch isn't going to handle the case of a single commit that references multiple issues. I do that a fair bit -- e.g. if there's a separate issue to fix a bug introduced in an earlier feature, I do stuff like:

To me, that commit is relevant to both #1234567 and #1234000. Not sure if it should generate an auto-comment in both -- it probably should. Certainly I'd want to see the commit in the block of commits for both issues.

Left some @todos which marvil07 sounds like he is willing to fix.

EDIT: missed the $operation['message'] in preg_match_all()

What's new on the patch:

• Minor change on the regular expression: add word boundary at the end.
• Make the comment aware of the operation autorship.
• Use theme_username.
• Use operation repository webviewer_url_handler plugin to generate the link to the commit.
• Get label names to show.
• Use check plain's as dww suggested.

Also adding a diff against last patch state.

I also did not really tried this patch on a proper environment :-(

Assume you wanted $label_names[] = $label->name;

Have to see how this works out after groups.drupal.org/node/161659, but probably fine for now.

I believe this is where dww wanted check_plain which makes sense.

Powered by Dreditor.

Seems good and agree we need to test somewhere. Not sure how we proceed.

Really happy to see this being picked up...but unfortunately, the approach here is wrong. Implementing the insert hook is not the way to do it - that hook is fired when a commit is recorded in the database, not when it initially arrives into the repo. The latter necessitates the former, but the former does not imply the latter - when we resync a repo (which we do fairly regularly for various reasons), we blow away all commit records then rebuild them from scratch. So every single commit in the history gets reinserted, and this hook is retriggered for each one. Imagine the cavalcade of automated issue comments :)

For when the commit truly first arrives in the repo, we have #1092062: Introduce framework for generic repository history synchronization. There are a couple smallish bugs related to merging that back in, but at this point the only thing that's blocking it is the fact that I don't have a git-dev to really test it on properly. re: that, I got git-dev just about set up again three weeks ago, but did it on an old Gentoo vm, instead of the new centos stagingvm2. Which is now temporarily borked because of an internal OSL/yum repo availability question. That's supposed to be resolved today, at which point I can resume efforts to bring git-dev back online.

That being said, I expect most of the actual logic here can be picked up and dropped into another hook implementation (and eventually shifted into a reposync event plugin). The hook to be implemented will be hook_versioncontrol_code_arrival(VersioncontrolRepository $repository, VersioncontrolEvent $event) (invocation here).

While I'm wearing my security team hat, it occurs to me that we should be checking node_access() here, too. Just because I put "#12345" into a commit message and could push that to your site doesn't mean I should actually have permission to view or comment on issue #12345. So, after node_load() we should both check $node->status and call node_access().

Agreed. And another reason why the event is very important - the relevant user to check for perms is *not* the commit's author/committer, but the person responsible for the network operation (the push). Which is not really accessible from the commit itself in its insert hook, but is a built-in part of VersioncontrolEvent: http://drupalcode.org/project/versioncontrol.git/blob/gsoc-activity:/inc... - though we should really add a getter method for it and maybe return a full user object.

And, reading more/further back - my ultimate preference would be to thread the commits in chronologically with the comment times as separate bits of information. That is what github does, though they go by committer timestamp, which changes when you rebase to the time you rebased, so commits almost always end up stacking in a single place in their interface rather than being interleaved. That'll be much more useful in a per-issue repo/pull request context, though, so I see no problem with auto-commenting as an intermediary solution, as getting together a good framework for interleaving the data could be a bit complex.

As for highlighting names in the commit messages...well, maybe. That data is a bit fuzzy, though, and I'd prefer to either a) read it from git-notes or b) use actual commit metadata to show who authored/committed the commit or c) just not highlight them at all. If we want to do that highlighting, we'd need a whole separate place to store these db records with the commit message analysis already performed, otherwise we'd be doing potentially expensive mysql searches for username matches on the fly. Minimum one string-based matching query against {users} per commit shown on a page.

@dww: thanks for the clarification :-), so yeah, we need to remove it for strings that are then used in theme('username') (hint: not the last one)

@sdboyer: agree on waiting for git-dev to test history sync branch properly and use pusher to review permissions instead of author/committer. agree on using a plugin to implement this logic so we can be more granular too for d.o and easily change it when git notes was official attribution.
IMO boombatower is right on his comment about highlighting messages, we are using what's on the operation itself already(committer_uid/author_uid), and that's is performed once(on the history sync hook world) just before saving the comment.

Other notes:

• I forgot the git backend store emails on author/committer operation data members, which means that we need to use author_name/commiter_name stored by git backend on its table for operation.
• Random though(almost offtopic): maybe a versioncontrol_project_git module is needed to separate specific logic for git(i.e. use author_name/commiter_name directly)
• Other solution(optimal IMHO) to avoid wait for testing on a staging site is to create more tests here which tries tricky(I mean involves lots of dependencies) things like this feature so we do not depend on it to add features to modules.

In conclusion, I guess we need to postpone this :-/