Edgewall Software
Modify

Opened 5 years ago

Last modified 5 years ago

#12907 new enhancement

Avoid duplicate references by commit_updater

Reported by: Raphael Geissert <atomo64+trac@…> Owned by:
Priority: normal Milestone:
Component: ticket system Version:
Severity: normal Keywords:
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Description

When creating new branches out of a git repository with commits with references to ticket, CommitTicketUpdater will happily add new, duplicate, comments.

Attached patch for 1.0 adds two features along with their toggles :

  1. detection of comments that already reference the changeset that is being added, and skips the update in that case. So no new comment nor email notification;
  2. and the disabling of email notifications when a changeset references to a closed ticket. The comment in the ticket is added however.

Point number 2 is specially helpful for the case where an svn repository is migrated to git: even though it is nice to have the reference to the git commit, it's useless to spam people about ancient commits.

Attachments (2)

commit_updater_dups.patch (4.9 KB ) - added by Raphael Geissert <atomo64+trac@…> 5 years ago.
commit_updater_dups.2.patch (4.1 KB ) - added by Raphael Geissert <atomo64+trac@…> 5 years ago.
v2

Download all attachments as: .zip

Change History (8)

by Raphael Geissert <atomo64+trac@…>, 5 years ago

Attachment: commit_updater_dups.patch added

comment:1 by Raphael Geissert <atomo64+trac@…>, 5 years ago

If the general idea and implementation are acceptable, I could look into updating it for 1.3. Based on a quick look it doesn't seem like there were a lot of changes around commit_updater.

N.b. any possible copyright of the new code added by the patch is copyright EDF S.A.

comment:2 by Jun Omae, 5 years ago

I don't think _is_duplicate should be renamed. The changes would break a component which is inherited from CommitTicketUpdater. At least, my private plugin wouldn't work after the patch.

by Raphael Geissert <atomo64+trac@…>, 5 years ago

Attachment: commit_updater_dups.2.patch added

v2

comment:3 by Raphael Geissert <atomo64+trac@…>, 5 years ago

I found it to be confusing to have an _is_duplicate that does less than the newly introduced _changeset_referenced. The newly attached patch keeps the original name.

in reply to:  1 ; comment:4 by Ryan J Ollos, 5 years ago

Replying to Raphael Geissert <atomo64+trac@…>:

N.b. any possible copyright of the new code added by the patch is copyright EDF S.A.

We don't have a formal contribution policy, but for anything other than large contributions we don't include copyright information in the file, so for us to include in the project I think you'll have to be willing to grant the contribution to the Trac project.

in reply to:  4 ; comment:5 by Raphael Geissert <atomo64+trac@…>, 5 years ago

Replying to Ryan J Ollos:

We don't have a formal contribution policy, but for anything other than large contributions we don't include copyright information in the file, so for us to include in the project I think you'll have to be willing to grant the contribution to the Trac project.

Perhaps I should have phrased it differently: if the small code portion that is being contributed is deemed to be copyrightable, it would be copyright EDF S.A. and made available under the same licence as the original code itself.

I'm not in a position to judge whether it is copyrightable, but the worst case scenario being "as the original code itself" isn't that good enough?

in reply to:  5 comment:6 by Ryan J Ollos, 5 years ago

Replying to Raphael Geissert <atomo64+trac@…>:

I'm not in a position to judge whether it is copyrightable, but the worst case scenario being "as the original code itself" isn't that good enough?

Yes, I think that's fine.

I haven't encountered the duplicated refs, so I'll have to try reproducing. Let me confirm the behavior, but then we'll probably need some unit test coverage in order to commit the patch.

The issue may also be fixed by #12485, but that ticket will require major changes so it's unlikely a near-term solution.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The ticket will remain with no owner.
The ticket will be disowned.
as The resolution will be set. Next status will be 'closed'.
The owner will be changed from (none) to anonymous. Next status will be 'assigned'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.