[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: RFC round 3: DEP-3: Patch Tagging Guidelines



Hi,

On Tue, 30 Jun 2009, Charles Plessy wrote:
> * “The meta-information would be stored in a set of RFC-2822 compliant fields.”
> 
> RFC-2822 distinguishes between “unstructured” and “structured” fields. The
> ‘Description’ field you propose is not unstructured, since the first line has a
> special role, but is not structured in the RFC-2822 way, since that would mean
> for instance that things between parenthesis are comments. Moreover, having a
> special role for the first CRLF sign is not consistant with the folding rules
> of the RFC. Given that in addition RFC-2822 is non-free and therefore can not
> be distributed in the Debian operating system, I think that it would be safer
> to write “in the spirit of the RFC-2822” of even “in the spirit of the Debian
> control files”, and write black on white what parsers should expect. Then we
> can point to DEP3 in DEP5 ;) 

I prefer discussing a real patch here. I think that I communicated the
intent clearly and I am not sure that I want to redefine the
debian/control format. Maybe we can point to manual page instead.

Maybe adding a few samples would be much more helpful in disambiguating
the syntax.

> * “In the following section, <Vendor> can be ‘Debian’ or the name of any other
>   distribution that tracks the same problem/patch.”
> 
> When I packaged my first Perl library, I really had a hard time to understand
> what “Vendor” was meaning and that Debian is a vendor since we do not sell our
> packages. If native English speakers confirm that a vendor is a seller, then I
> would like to suggest to use another keyword, like “Organisation”.

I want to standardize on vendor since we have dpkg-vendor now.

> * “Origin (required): This field should document the origin of the patch. It
>   starts with a single keyword that can have the following standard values”
> 
> I think that the rules are too hard to follow strictly. What if the patch is
> not from upstream but for backporting, for instance?

That case is covered: « “backport” (in the case of an upstream patch that
had to be modified to apply on the current version) »

The rules are not hard and if you don't know you should default to
"other". We could make that explicit if really needed.

> * “The Bug field is reserved for the bug URL in the upstream bug tracker.”
> 
> I think that parsers can be educated to be smart with URLs (like for
> bts-link?). I would prefer to just cut-and-paste any URL to the Bug field.

It could but it's a serious regression as I stated since you can't be
smart enough to differentiate the upstream bug in non-ambiguous way.

> * “If the [Forwarded] field is missing, its implicit value is ‘yes’ if the ‘Bug’
>   field is present, otherwise it’s ‘no’”
> 
> I am affraid it will create false positives and recommend an implicit value of
> ‘no’ in all cases.

I don't agree with that. If you know the upstream bug, you did read it and
you know if the patch is referenced there. If you know it has not yet been
sent there and you don't send it you're a very bad maintainer (it's cheap
to forward the patch once you located the uptream bug entry!). If you're a
bad maintainer, you don't even care about writing patch meta-information
in the first place.

In other words, the risk of false positive is very low IMO.

Furthermore I want to avoid duplication of information in the fields and
if you default to "no" then you have to put "Forwarded: yes" whenever you
add "Bug: …" because you forwarded the patch by creating a new upstream
bug entry.

In the balance, I prefer the risk of false positive compared to the risk
of making it laborious/non-intuitive to get the correct set of fields.

> * “Reviewed-by (optional): This field can be used to document the fact that the
>   patch has been reviewed by someone. It should list her name and email in the
>   standard format”
> 
> How about relaxing the format so that the field can also point at the review
> in the case it makes sense reading it?

Yes, we could use continuation lines for that. It could directly contain
the review or point to an external resource.

> I think I will not use this field: it has too much chances to be inconsistent
> by accident, and most of the time this information is available in the VCS
> where the source package is stored.

Fine with me, it's optional on purpose.

Cheers,
-- 
Raphaël Hertzog

Contribuez à Debian et gagnez un cahier de l'admin Debian Lenny :
http://www.ouaza.com/wp/2009/03/02/contribuer-a-debian-gagner-un-livre/


Reply to: