hi raphael,
On Wed, Jul 15, 2009 at 11:16:01PM +0200, Raphael Hertzog wrote:
<snip discussion regarding freetext description location>
> How do you expect to recognize the real starting point for the fields?
> The freetext might contain text that look like field names at the start
> of a line... I don't think that requesting fields to be first in the patch
> file (except shebang lines) is a real burden for maintainers...
>
> What do others people think ?
i should say that i don't feel incredibly strongly about this, i was merely
postulating that supporting something in the form of
<possible description block>
<blank line>
<headers block>
wouldn't be so different than
<headers block>
<blank line>
<possible description block>
i.e.:
while read_a_series_of_uninteruppted_lines_from_metainfo():
if block_is_all_headers():
process_headers()
else
append_to_description()
but the more i think about it the more i think that it's not really worth
the effort, and forcing at least *some* modicum of structure could arguably
be a good thing anyway.
<snip discussion wrt reviewed by as single or split address/date fields>
> Given that we can have multiple Reviewed-by fields, how would
> both fields be linked together?
i should say that i also don't have a strong opinion on this either,
i only commented because i thought that (a) having timestamps could be
valuable and (b) using two fields/lines might seem more natural for reading
and writing by humans.
with regards to linking them together, i suppose it'd be a convention
that the most recent "reviewed date" referred to the most recent "reviewed by"
field.
> I think we should rather recommend to include a timestamp in the
> review itself (supplementary lines in the Reviewed-by field?).
so instead of:
Reviewed-By: Some Reviewing Person <email@address.here>
Reviewed-Date: <date in some agreed format>
you suggest something like:
Reviewed-By: Some Reviewing Person <email@address.here> on <date in some agreed format>
or in the case the user wanted to break into multiple lines:
Reviewed-By: Some Reviewing Person <email@address.here>
on <date in some agreed format>
that doesn't seem so bad...
> Is it important to be able to automatically extract that information
> or is that mainly for the maintainer's consumption?
in an ideal world i'd say both (i.e. detecting stale review info).
but there's also an extra manual step which wouldn't be addressed by
either solution, for either the human or the automated script: "has the
patch been updated since it was reviewed?". it would require diving
through the VCS and/or reading changelogs to determine if a patch was in
the same state as when it was reviewed (i think trusting local stat(2)
info on the file is not a good solution).
or, since i've reached the bottom of the mail here, but still want to
procrastinate doing any real work today, here's a thought that
just bubbled up: what if the reviewed information included some kind of
patch body checksum to determine whether a patch was modified?
perhaps there could be a better way of saying it, but i'm thinking
something along the lines of:
Reviewed-By: Some Reviewing Person <email@address.here>
on <date in some agreed format>
for <md5sum>
where <md5sum> could be generated by taking the md5sum of the patch body
with all leading metadata stripped, possibly using a small utility tool
for the lazy[1].
in any case, an automated lintian check could then detect when a patch
had such a field that was out of date. Of course i'm not convinced that
it'd be so useful that people would actually *use* such a feature...
sean
[1] this still doesn't address anything wrt authenticity, it's only
convenience information.
Attachment:
signature.asc
Description: Digital signature