Hi Guillem,
> I'm wondering why diverge from the patch metadata guidelines? If there's
> a desire to change the field semantics, perhaps it would be better to
> change the guidelines instead? :)
>
> But given this interchange, perhaps we should try to make it more
> clear or explicit about some of these aspects there!
Originally I admit that I misread the DEP3 rules.
Now the implementation is much closer to DEP3. Unfortunately, DEP3 was
probably not written with automatic processing in mind.
The current status of the data is the following[1]:
forwarded_short | explanation
| cnt | percent
-----------------+------------------------------------------------------------------------------+--------+---------
no | No Forwarded, No Bug field
| 100145 | 68.12
not-needed |
| 19329 | 13.15
yes |
| 12217 | 8.31
no | Explicit Forwarded=no
| 11448 | 7.79
invalid | Invalid value for Forwarded field
| 2270 | 1.54
invalid | Forwarded=yes, but no Bug field
| 1119 | 0.76
invalid | No forwarded, Debian bug in Bug: field. Bug-Debian: should
be used instead. | 283 | 0.19
no | No Forwarded, Upstream bug, but not a URL
| 190 | 0.13
invalid | Forwarded=yes, Debian bug in Bug: field. Bug-Debian: should
be used instead. | 8 | 0.01
With some comments:
no | No Forwarded, No Bug field
| 100145 | 68.12
=> that sticks to the DEP3 specification. (no forwarded: + no bugs: => implicit
no)
not-needed |
| 19329 | 13.15
=> that's an explicit "Forwarded: not-needed"
yes |
| 12217 | 8.31
=> Those are the cases where it's clear that the bug has been forwarded
no | Explicit Forwarded=no
| 11448 | 7.79
=> that's an explicit "Forwarded: no"
The remaining lines are the ones that could require discussion.
First, it's worth noting that they only account for 2.63% of the
patches.
invalid | Invalid value for Forwarded field
| 2270 | 1.54
=> I looked at the values considered invalid[2], and could not really convince
myself that they should automatically fit in one of the above
categories.
invalid | Forwarded=yes, but no Bug field
| 1119 | 0.76
=> I think that we should strongly encourage leaving a public trace when
forwarding bugs
invalid | No forwarded, Debian bug in Bug: field. Bug-Debian: should
be used instead. | 283 | 0.19
=> quite obviously invalid
no | No Forwarded, Upstream bug, but not a URL
| 190 | 0.13
=> those are mostly patches misusing the Bug: field for the Debian
bug.[3]. Since the specification requires a URL here, it could be
considered invalid as well.
invalid | Forwarded=yes, Debian bug in Bug: field. Bug-Debian: should
be used instead. | 8 | 0.01
=> quite obviously invalid
So, all in all, my implementation[4] sounds sane.
> I otherwise do not know how we can mark patches as forwarded when
> for example you send them directly to upstream via email or to a mailing
> list that has no public archive or similar.
That's true. Other than blinding accepting "Forwarded: yes", I don't
know how this could be addressed.
> I think the Origin field needs to be taken into account too, in
> particular when it has an "upstream" or a "backport" value. As well as
> the Applied-Upstream field.
I would prefer the Forwarded field to be a simple field only allowing
'no', 'not-needed', and maybe 'yes' (with the pointer to where the patch
was forwarded in a separate field).
Or if the Forwarded field's value is supposed to be computed, I think
that we should provide a more detailed specification of the algorithm.
- Lucas
[1] UDD query:
SELECT forwarded_short, invalid_reason, cnt, round(100.0 * cnt / (sum(cnt) OVER
()),2) as percent
FROM ( select forwarded_short, invalid_reason, count(*) cnt from patches group
by 1,2 ) foo
order by 3 desc
[2] UDD query:
select forwarded, count(*) from patches where invalid_reason='Invalid value for
Forwarded field' group by 1 order by 2 desc;
[3] UDD query:
select forwarded, upstream_bug, count(*) from patches where invalid_reason='No
Forwarded, Upstream bug, but not a URL' group by 1,2 order by 3 desc;
[4] https://salsa.debian.org/qa/udd/-/blob/master/rimporters/patches.rb#L197
