patchew-project / patchew

A patch email tracking and testing system
MIT License
72 stars 24 forks source link

Patchew tags include some false positives #135

Closed jnsnow closed 2 years ago

jnsnow commented 2 years ago

I wrote a little script to try and understand the tags field that patchew uses, and found a few things that didn't match a "X: Y" format, like these:

Unrecognized tag format 'Reviewed-by when resending. Please do if there is a v5. I will'
Unrecognized tag format 'Tested-by tag, so I haven't included it in the patches.'
Unrecognized tag format 'reviewed-by stamp.'
Unrecognized tag format 'Reported-by tag is used instead to note who reported the problem but I '
Unrecognized tag format 'Acked-by tags it will make my life easier because the tooling will do'
Unrecognized tag format 'Tested-by tag due to the extra PROM changes, so I'd be grateful if you could give it '
Unrecognized tag format 'Reviewed-by/Acked-by tags to the corresponding ones (these'
Unrecognized tag format 'Reviewed-by and wait anyway for others to chip in.'

This suggests that patchew is erroneously picking up some tags that nobody meant to actually apply to the commit. I am ... not actually certain if the hosted patchew instance is still doing this, or if this just remnants from an older version, though.

famz commented 2 years ago

Hi John,

Good catch. It seems there is a bug in the tag extraction code. I'll take a look. Thanks!

famz commented 2 years ago

I think if the tags in the configuration doesn't have a ":" in the end, this may happen. The default behaviour is okay because we have

REV_BY_PREFIX = "Reviewed-by:"
BASED_ON_PREFIX = "Based-on:"
SUPERSEDES_PREFIX = "Supersedes:"

...

BUILT_IN_TAGS = [REV_BY_PREFIX, BASED_ON_PREFIX, SUPERSEDES_PREFIX]
jnsnow commented 2 years ago

hi @famz !

If I understand you correctly, this is probably only a cached result from older patches prior to the config being updated, right? If so, you could probably just close this issue.