On Sun, Oct 16, 2011 at 4:57 AM, Stefan Richter <stefanr@xxxxxxxxxxxxxxxxx> wrote: > Hi Piotr, > > thanks for getting this going again. - I have not yet looked through the > source but have some small remarks on the patch format. > > - In your changelogs and in the diffs, somehow the space between real > name and e-mail address got lost. > > - The repetition of the Subject: line as first line in the changelog is > unnecessary (and would cause an undesired duplication e.g. when git-am > is used, last time I checked). > > - AFAICT, author of patch 1/7 is not Devin but you. Hence the From: line > right above the changelog is wrong. > > - The reference to the source hg tree is very helpful. However, since > the as102 related history in there is very well laid out, it may be > beneficial to quote some of this prior history. I suggest, include > the changelog of "as102: import initial as102 driver", > http://kernellabs.com/hg/~dheitmueller/v4l-dvb-as102-2/rev/a78bda1e1a0b > (but mark it clearly as a quote from the out-of-tree repo), and include > a shortlog from this commit inclusive until the head commit inclusive. > (Note, the hg author field appears to be wrong; some of the changes > apparently need to be attributed to Pierrick Hascoet as author.) > This would IMO improve the picture of who contributed what when this > goes into mainline git history, even though the hg history needed to > be discarded. > > - A diffstat is always very nice to have in a patch posting. Most tools > for patch generation make it easy to add, and it helps the recipients > of the patch posting. > (Also, a diffstat of all patches combined would be good to have in the > introductory PATCH 0/n posting, but not many developers take the time > to do so.) > > Again, thanks for the effort and also thanks to Devin for making it > possible. I think collapsing my entire patch series in to a single patch would not be acceptable, as it loses the entire history of what code was originally delivered by Abilis as well as what changes I made. The fact that it's from multiple authors (including a commercial entity contributing the code) makes this worse. I think the tree does need to be rebased, but I don't think the entire patch series would need to be reworked to be on staging from the beginning. You could probably import the first patch (as102: import initial as102 driver), fix the usb_alloc_coherent() so that it compiles (and put a note in it saying you did), apply the rest of the patch series, and then add a final patch that says something like "moving to staging as code is not production ready". This would allow the history to be preserved without having to rebase every patch to deal with the files being moved to the staging tree. An alternative would be make a minor tweak to my first patch which removes the driver from the makefile. Then every patch in the patch series wouldn't actually have to compile successfully until the very end when you add it back into the Makefile. This is a luxury you can do when it's a brand new driver. When it's a brand new driver there is a bit more flexibility as long as you don't break "git bisect". Both of the approaches described above accomplish that. Mauro, what do you think? Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
