1173560 - De-duplicate bug numbers in the newly added metadata shown on hgweb

Status: REOPENED

(Developer Services :: Mercurial: hg.mozilla.org, defect)

Description

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

The metadata shows the same bug number multiple times if it finds multiple instances in the commit information (eg https://hg.mozilla.org/integration/b2g-inbound/rev/dbf3b1ad8240 shows "bugs: 30507, 1169861, 1169861, 1169861")

It'd be nice if it would only list each bug once no matter how many times it finds it in the commit.

Comment 1

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Attachment: Pass the list of bugs through set() to remove duplicates.

I think this is all that would be needed?

Comment 2

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Or should this be on the commit parser's side?

Comment 3

[Gregory Szorc [:gps]]

Comment on attachment 8620646 [details] [diff] [review]
Pass the list of bugs through set() to remove duplicates.

Yes, this is all that is needed. Although commit message order may get lost. I don't think we care though.

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/hgcustom/version-control-tools/rev/0ac18a3c3443

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Hrm, it's still listing duplicates. Do I need to get rid of the list() part to make sure set() remains a unique list?

Comment 6

[Gregory Szorc [:gps]]

Look at the line below. Why we're making two calls to parse bugs, I don't know. How I missed this in code review, I can't believe.

This is why you write tests.

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Attachment: MozReview Request: Bug 1173560 - Really remove duplicate bug numbers from the metadata before it gets displayed.

Bug 1173560 - Really remove duplicate bug numbers from the metadata before it gets displayed.

For some reason, there were two attempts to parse bugs, but it looks like only the second attempt is actually used for anything, while my first patch only changed the first attempt.

This version removes that first, apparently unused parse, and removes the duplicates from the actually-used parse.

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

^ is totally untested by me. Don't know if calling set() like that changes things too much beyond what hg expects. I'm also unsure exactly how to test it. Please don't let me land this without someone making sure it doesn't break everything. :)

Comment 9

[Gregory Szorc [:gps]]

Comment on attachment 8621242 [details]
MozReview Request: Bug 1173560 - Really remove duplicate bug numbers from the metadata before it gets displayed.

https://reviewboard.mozilla.org/r/10925/#review9855

::: hgext/hgmo/__init__.py:33
(Diff revision 1)
> +    d['bugs'] = set(d['bugs']);

We prefer to keep the ordering of this list. Plus, doing a set() on a list of dicts doesn't work because set() works by looking at hash(<object>) and hash({}) doesn't work. I suspect you would have discovered this had you ran the tests:

  $ ./run-tests hgext/hgmo
  
You'll want to introduce a new variable to track which bugs have been seen. e.g.:

  d['bugs'] = []
  seen_bugs = set()
  for bug in ...:
      if bug in seen_bugs:
          continue
      seen_bugs.add(bug)
      d['bugs'].append({...})

::: hgext/hgmo/__init__.py:41
(Diff revision 1)
> +    d['reviewers'] = set(d['reviewers']);

Ditto.