here's a nice big group-reply with an assortment of comments :) overall i think this looks pretty good, though i do have some concerns that will kind of repeat themselves below about there being too much emphasis on something machine friendly instead of human friendly... On Sun, Jun 21, 2009 at 12:07:42PM -0500, Raphael Geissert wrote: > Raphael Hertzog wrote: > > > Origin (required) > Making this field mandatory doesn't sound like a good idea to me, as it ACK On Mon, Jun 29, 2009 at 10:03:24PM +0200, Raphael Hertzog wrote: > On Sun, 21 Jun 2009, Raphael Geissert wrote: > > > Description (required) > > Why not simply consider all the free-form text the description? that would > > make all the current patches with a comment insta DEP3-compliant. > > Done, but that's a recommendatino for the parser. Note that it's not > DEP3-compliant since the Origin field is required. i really don't see why the origin field should be required, at least as is. personally i'd find the idea a bit cumbersome, as the same "keyword" information has probably been documented elsewhere (the changelog) and possibly duplicated (in the patch description), maybe even multiple times (the version control system), and possibly incorrect (a cherry-picked patch is updated locally due to other conflicting patches, but the header still implies otherwise because the maintainer forgot to update it). i'm also not sure i see the benefit in being able to know in an automated and machine readable way whether a patch was "cherry-picked" or "backported" or "cross-ported" or from an arbitrary "vendor" or "other". so... why not just let it be a URL or textual description? > > > Origin (required) > > Making this field mandatory doesn't sound like a good idea to me, as it > > already clashes a bit with the forwarded and author fields. If the Origin > > is upstream, then it doesn't need to be forwarded; and it doesn't cope very > > well with the idea of patches by some John Doe user. > > I believe it's important to be able to know where the patch came from. I do think that knowing where the patch came from is very important, and one of the main driving rationales behind this proposal. but more than a URL or a revision/commit id, and something that might need to be kept up-to-date? what's the benefit of having it be in such a strict and machine readable format? what benefit will a parser provide us being able to figure this out over us just reading it ourselves? (i'm not trying to be rhetorical i'm actually interested in hearing the justification) > > > Bug-<Vendor> or Bug (optional) > > Like Paul Wise already said: it would be better to have a single field where > > the urls to the bug trackers can be specified. It doesn't only make it > > easier to find the final url, but it also requires zero extra > > maintenance/updates on the parsing tools just to know about another bug > > tracker. > > Paul did not say that, he simply told that he preferred URLs instead of > bug numbers. also, it should be kept in mind that some upstreams do not have bug tracking systems at all, so the URL could very well be referencing a mailing list post or news update or similar. > Are you saying that you don't want Bug-<Vendor> but only Bug without > any requirement to indicate the vendor ? > > I think it would be bad because it would not allow to differentiate the > upstream bug url from the others. is there a benefit to differentiating in a machine readable way? if a human reads that, they should by context be able to tell which references the upstream (i.e. bugs.project.net), vs. debian (i.e. bugs.debian.org) vs some other vendor just by reading it. if there's a rationale, i think it should be included in the DEP to clarify why this is important. for example, is it so that the patch can be traded between distros with minimal fuss to the headers? On Tue, Jun 30, 2009 at 08:49:21AM +0200, Raphael Hertzog wrote: > > * “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) » (see above question about benefit of strict Origin syntax) > The rules are not hard and if you don't know you should default to > "other". We could make that explicit if really needed. at the very least, could "other" be implied with the lack of this field? i.e., it seems much more natural to say Origin: http://blah/foo.html and allow the keywords like "upstream" to be used as optional sugar to add further information. and what about Origin: Some User <email@fqdn> okay, maybe that should be Author, but then why have an additional and duplicate field "Origin: other, submitted by..." requirement? later on, On Wed, Jul 01, 2009 at 02:08:28PM +0200, Raphael Hertzog wrote: > That said, supporting the "patch as script" case needs some trickery to > be able to reuse existing parsers (stripping "# " before passing lines > to the parser). But allowing invalid lines as comments in the middle will > make it completely impossible to reuse standard parsers. what about allowing the freetext preceeding or following the fields, but specifying the fields are to be uninterrupted? normalizing that into something that you could throw at a standard parser is only a handful of lines of code at most, and if you're already doing some trickery wrt dpatch's '#' that's a pretty marginal cost. On Fri, Jul 03, 2009 at 02:07:15PM +0100, Jon Dowland wrote: > One thing I would like to see patch metadata help > facilitate is patch review. At the moment the "Reviewed-by" > header proposed would allow a tool to ensure at least two > sets of eyeballs had seen a patch; however, patches can go > stale. I think some chronological information is needed > alongside the review. I propose a "Last-Reviewed" header to > capture this information. seems reasonable... On Fri, Jul 03, 2009 at 02:36:48PM -0400, James Westby wrote: > > Structure > > This certainly makes it easier to implement parsers, but I'm wary of it > being too strict. there's a trade off between making it easier for machines vs easier for developers and other human reviewers etc. if it's not already clear from my above interjections, i'm siding towards the mere mortals, as i think it provides clearer and more tangible benefits. i think a more detailed out list of the benefits of the contrary would be good to bolster the other side of this argument. > Is it worth advising that lines be < 80 characters (including > "Description: ")? i'd say so, but only as a polite suggestion/reminder. > > Josselin Mouette wanted to allow bug numbers instead of URLs in the Bug-*/Bug > > I don't like this suggestion at all. Copying and pasting a URL in is > generally convenient, and while many of us will have quick ways of going > from a bug number to the bug page, new contributors and those outside > the project won't, and so it will be harder for them to get the > information. it's worked in changelogs up to this point... also, it's quite possible that they might be reading this through some kind of "patch tracking system", in which case the machine can have already figured out the URL and made the bug number a hyperlink to it :) sean --
Attachment:
signature.asc
Description: Digital signature