Commit notification comments are added for commits referencing issues in topic branches event when those commits are not intended as final/ready to integrate in main branch.

Proposed resolution

Add a new hook alter to let any module modify the issues to add a comment on.

Remaining tasks

Test code.

User interface changes


API changes

New hook alter.

Original report by @drumm

The drupalorg module started using Feature branches, and the comments on commits have gotten quite chatty.

A couple rules I think could help:

  • If a commit is on a standard branch name, like 7.x-3.x, keep notifying as we do today. (We can't look for branches being associated with releases, since the commits are still important if the dev release node hasn't been made.
  • Otherwise, only comment if the issue's node ID is also in the branch name. This will notify for feature branches that follow the convention of including the ID.

This should keep good notifications for the popular always commit to HEAD workflow. When feature branches are used, we'll notify for commits with both the message and branch name that have the issue ID, and again when merged into a branch for releases.


marvil07’s picture

This sounds like a good idea, but I would say it is really specific to d.o, so let's create a hook to let modules, e.g. drupalorg_versioncontrol add extra logic when needed.
This issue could be about that, and we could create a related one for drupalorg.

jhodgdon’s picture

+1 to the logic. Implementation, I have no opinion on but marvil07's idea sounds like a reasonable idea.

marvil07’s picture

Title: Be more selective adding commit comments on branches » Let other modules modify the target issues to comment
Issue summary: View changes
Status: Active » Needs review
Related issues: +#2227411: Commit backlink incorrectly appears when commit made to new branch, +#493074: Back-link to the commit as a comment on the related issue.
2.95 KB
1.76 KB

Here an initial try.

In the process I discovered some bugs, here related commits just for reference(I failed to create yet another issue for it):

    Issue #2227411 follow-up: Some extra fixes.
    - Typo on watchdog message.
    - filterRelevantOperationLabelNames() method call does not need to be
      executed per project issue loaded.
    - Only by pass current ref on getRelevantEventLabelNames() if not a
      branch, i.e. not all refs in the event.

Attached the patch which adds the new hook alter.
Also attached the drupalorg patch implementing the new hook alter(someone please create that issue in drupalorg project issue queue). Pending there the filter to remove issue if branch name does not contain the issue nid.

I did not yet tried the code, @drumm, any comments on the logic at the hook implementation? (mainly from drupalorg_versioncontrol/plugins/label_version_mapper/DrupalorgVersioncontrolLabelVersionMapperGit.class.php)

marvil07’s picture

Title: Let other modules modify the target issues to comment » Let other modules modify the target issues to action into
Assigned: Unassigned » drumm
Issue summary: View changes
Related issues: +#2363121: Restrict action into issues after versioncontrol_project_issue_git event processors
5.61 KB
5.97 KB

I found another kind-of-related little bug:

  • 2ee5339 Check empty passed array in versioncontrol_project_issue_get_issue_operation_ids() before passing it to a query.

I have generalized a little to let pass the plugin name, so this hook can be call from both git_commits_as_comment and git_issue_mapper.

This time I could actually test/run the code and it seems to be working OK.

I created a ticket for drupalorg related code: #2363121: Restrict action into issues after versioncontrol_project_issue_git event processors

I think this part of the code is ready, @drumm, let me know if you want to review it before I add it.

  • marvil07 committed 1785278 on 7.x-1.x
    Issue #2360989: Let other modules modify the target issues to action...
marvil07’s picture

Assigned: drumm » Unassigned
Status: Needs review » Fixed

After a quick chat with drumm in IRC, this is now added to upstream.

marvil07’s picture

adding tag

drumm’s picture

Now deployed on

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.