Raphael Hertzog wrote: > I just gave a quick look to your branch: I can understand why all these might be best practices for submitting patches via git for a project like the linux kernel, but dpkg is not exactly drowning in perfectly-presented git patchsets, and IMHO there are good reasons to ignore every suggestion you made. > - please rebase it on the current master branch (that way you're sure that > there are no conflicts) I have yet to see a use of git rebase that I am comfortable with. History is very important to me, even the ugly bits. Being able to see every misttep that was committed actually helps understanding the end result. Merging it with the current upstream master would have the same effect with a more "ugly" history, surely? > - please update your git identity, I see "Ian Jackson > <ian@anarres.relativity.greenend.org.uk>". You should standardize > your contributions to a single email. FYI, your previous patches have been > applied as "Ian Jackson <iwj@ubuntu.com>". You can use git-filter-branch > to rewrite that field automatically. Again, history is important to me, in this case I quite like that git preserves a history of where I did my work, so I have no interest in using a single email address. (AFAIK, this isn't even a general best practice. For example Linus commits from linus@woody.linux-foundation.org and signs off patches from linus@linux-foundation.org. Earlier in the kernel history, it's torvalds@ppc970.osdl.org.) > - please update the commit logs, a copy/paste of the ubuntu changelog > doesn't look good to me, follow the recommandation of using > a summary in the first line, then an empty line, then a long > description. > see http://wiki.debian.org/Teams/Dpkg/GitUsage > (this can be done during the interactive rebase process, see man > git-rebase and its -i option) Again, history is important to me, moreover, this is a lot of work for essentially zero gain. Yes, I'm lazy. If I have a perfectly ok changelog entry in debian/changelog, I will debcommit it, and not reiterate the same changes in a different style. > - you can also restructure the serie of changes with "git rebase -i origin/master" > (provided that origin/master is an up-to-date copy of the master branch > of the main dpkg repository) > (merge several commits in a single, in particular bug fixes with initial > implementation so that while reviewing diff you don't find bugs which > are in fact fixed by a subsequent commit) This loses history, is highly complex, requires a significant knowledge of git (more than I am comfortable with after using it for a month), and AFAICS is an anourmous time sink for no benefit. I'd rather be coding. > I can't judge the content but following the "right form" is more likely to > lead to a quicker review. If that's actually true of dpkg, I'll be sure to limit the information I publish about dpkg patches to unified diffs and avoid all this nonsense. As someone who needs this patch merged as soon as possible so we can start *using* it, your comments came off as somewhat tinged with obstructionism and git fanboyism. I'm sure you didn't intend that, but that was my impression. :-/ -- see shy jo
Attachment:
signature.asc
Description: Digital signature