Hi all,
There is something concerning submitting patches that bothers me to no end: inlining vs. attaching them.
I don't know about others, but for me 99/1 rule applies: I at least skim over 99% of the patches inlines in the message, whereas I bother to read at most 1% of the ones attached/tar.gzed/ziped. Maybe I'm an extreme case, but there's got to be more to it than my personal quirks.
If a patch is sent to wine-patches, it's sent there for peer review. If you don't want the review, send it to Alexandre directly, even though I suggest this is avoided as much as possible. However, it you do send it to wine-patches, please, *please* inline it!
What about a nice patch submission policy: -- unified diff only (required) -- have a decent subject (recommended) -- a long description (optional, if the change warrants it) -- a meaningful ChangeLog entry (required) -- new files, if any, included in patch, diffed against /dev/null (required) -- patch inlined at the end of the message (required) -- one changeset per message
Most of these things are already followed by most people, with the exception of the inlining bit. Alexandre, what about you reject patches that aren't in this format with a pointer to these rules? All this is a matter of habit, and I think we'll all benefit if these rules are followed.
"Dimitrie" == Dimitrie O Paun dpaun@rogers.com writes:
Dimitrie> -- new files, if any, included in patch, diffed Dimitrie> against /dev/null (required) --
How to proceed with a new directory?
On September 9, 2002 02:03 pm, Uwe Bonnes wrote:
How to proceed with a new directory?
Just make a small note for Alexandre, in the optional long description (the stuff that goes after Subject but before the ChangeLog entry) to add the directory(ies) before applying the patch.
"Dimitrie O. Paun" dpaun@rogers.com writes:
Just make a small note for Alexandre, in the optional long description (the stuff that goes after Subject but before the ChangeLog entry) to add the directory(ies) before applying the patch.
You don't even need that. Simply diff the new files against /dev/null, patch will create the needed directories automatically.
--- "Dimitrie O. Paun" dpaun@rogers.com wrote:
Hi all,
There is something concerning submitting patches that bothers me to no end: inlining vs. attaching them.
[skipped]
I have to attach the patches :-( because I use Yahoo online mail client and it wraps long lines. I also usually compress patches with size >= 20-30kb to save everybody's bandwith.
It is good idea to add these recommendations to the Starter's guide, even if we are not going to enforce them.
Andriy, Boston, MA
__________________________________________________ Do You Yahoo!? Yahoo! Finance - Get real-time stock quotes http://finance.yahoo.com
Andriy Palamarchuk wrote:
--- "Dimitrie O. Paun" dpaun@rogers.com wrote:
Hi all,
There is something concerning submitting patches that bothers me to no end: inlining vs. attaching them.
[skipped]
I have to attach the patches :-( because I use Yahoo online mail client and it wraps long lines. I also usually compress patches with size >= 20-30kb to save everybody's bandwith.
It is good idea to add these recommendations to the Starter's guide, even if we are not going to enforce them.
I think attachments are okay, as long as they are plain text. They do show up inline. The only problems for me are when the attachments are encoded, or when the attachment is identified as other than plain text (for example, those posted by PS). I think most email programs should be able to be configured to identify .diff files as plain text.
On September 9, 2002 02:30 pm, Andriy Palamarchuk wrote:
I also usually compress patches with size >= 20-30kb to save everybody's bandwith.
Please don't. Bandwidth is order of magnitudes cheaper than people's time, attention, *and* expertise. A skilled reviewer costs a LOT more than the few fraction of a penny you save compressing. It's simply a bad tradeoff.
Not to mention that if the patch is inlined, it gets nicely archived it the mail archives, indexed by search engines, etc.
-- Dimi.
On Mon, Sep 09, 2002 at 02:41:15PM -0400, Dimitrie O. Paun wrote:
On September 9, 2002 02:30 pm, Andriy Palamarchuk wrote:
I also usually compress patches with size >= 20-30kb to save everybody's bandwith.
Please don't. Bandwidth is order of magnitudes cheaper than people's time, attention, *and* expertise. A skilled reviewer costs a LOT more than the few fraction of a penny you save compressing. It's simply a bad tradeoff.
Not only that, most modem links will do a moderate amount of compression - so it isn't anywhere near as bad as it might seen.
David
"Dimitrie O. Paun" dpaun@rogers.com writes:
Most of these things are already followed by most people, with the exception of the inlining bit. Alexandre, what about you reject patches that aren't in this format with a pointer to these rules? All this is a matter of habit, and I think we'll all benefit if these rules are followed.
I don't want to make it a strictly enforced rule, because I think there is a risk of discouraging people, especially occasional contributors. After you have spent hours tracking a bug and writing a patch, it's already hard when the patch is rejected because it's wrong, but if it gets rejected without anybody even looking at it because you didn't use the proper format, that's very discouraging. I think we should try to follow the network rule: be conservative in what you send and liberal in what you accept.
I do admit that it would be nice if frequent contributors could get into the habit of making patches that are easier to read and apply. I'll also note that I tend to deal first with patches that don't require me to do MIME gymnastics, so these are less likely to end up getting forgotten. Maybe that could provide some motivation...
On September 9, 2002 02:38 pm, Alexandre Julliard wrote:
I think we should try to follow the network rule: be conservative in what you send and liberal in what you accept.
That's a good point. In fact, I wasn't that serious when I suggested making this a hard and fast rule, as I am aware of the dangers. I was thinking more in terms of *strongly* desired set of rules, rather then absolute. And applying more so to usual contributors, rather then the occasional one.
I do admit that it would be nice if frequent contributors could get into the habit of making patches that are easier to read and apply. I'll also note that I tend to deal first with patches that don't require me to do MIME gymnastics, so these are less likely to end up getting forgotten. Maybe that could provide some motivation...
Maybe accept the patch, but send a reply to the author asking to follow the rules, if possible? There are some cases where we have to make exceptions, but most people should be able to follow them...
-- Dimi.
I beg to differ.
When I copy/paste text, my mail compser feels it has the right (and for a good reason) to do stuff to the text. This may include changing tabs with spaces, word wraps, etc. These are exteremely annoying.
On the other hand, to the best of my knowledge, when I attach a text document, at least when I later view the message, the document is displayed inline (as opposed to actually being inline, which it isn't).
Dimitrie - please find one of my patch submissions, and let me know if it is displayed inline for you. If it is, attaching a file called .diff is the right way to go. If not, attaching .txt may be a solution. I am very much against copying the patch into the message, however.
Shachar
Dimitrie O. Paun wrote:
Hi all,
There is something concerning submitting patches that bothers me to no end: inlining vs. attaching them.
I don't know about others, but for me 99/1 rule applies: I at least skim over 99% of the patches inlines in the message, whereas I bother to read at most 1% of the ones attached/tar.gzed/ziped. Maybe I'm an extreme case, but there's got to be more to it than my personal quirks.
If a patch is sent to wine-patches, it's sent there for peer review. If you don't want the review, send it to Alexandre directly, even though I suggest this is avoided as much as possible. However, it you do send it to wine-patches, please, *please* inline it!
What about a nice patch submission policy: -- unified diff only (required) -- have a decent subject (recommended) -- a long description (optional, if the change warrants it) -- a meaningful ChangeLog entry (required) -- new files, if any, included in patch, diffed against /dev/null (required) -- patch inlined at the end of the message (required) -- one changeset per message
Most of these things are already followed by most people, with the exception of the inlining bit. Alexandre, what about you reject patches that aren't in this format with a pointer to these rules? All this is a matter of habit, and I think we'll all benefit if these rules are followed.
On September 10, 2002 12:55 am, Shachar Shemesh wrote:
I beg to differ.
I did not intend to start a flamewar -- various solutions to this very problem have been tried in the OSS comunity for a number of years, and it seems that people in the know prefer inlined patches (or attached as text). And this is for a lot of good reasons, some of which I have already mentioned. I should note that Linus will simply drop patches if they are not inlined.
When I copy/paste text, my mail compser feels it has the right (and for a good reason) to do stuff to the text. This may include changing tabs with spaces, word wraps, etc. These are exteremely annoying.
Most mailers have an "Insert file" action. Simply turn off line wrapping and there you go. If that doesn't work, your mailer is broken. It is also true that are a few mailers out there which are known to be broken. Some old pine versions, for example.
Dimitrie - please find one of my patch submissions, and let me know if it is displayed inline for you. If it is, attaching a file called .diff is the right way to go. If not, attaching .txt may be a solution. I am very much against copying the patch into the message, however.
You shouldn't be. If you can control your mailer, there's nothing wrong with them.
Your patches show inline, they are fine. If this is how you want to do it, that's fine. For all practical purposes, actually inlining the patch, or attaching it as plain text, works about the same. Pick your poison.
-- Dimi.
On Tue, Sep 10, 2002 at 07:55:51AM +0300, Shachar Shemesh wrote:
I beg to differ.
When I copy/paste text, my mail compser feels it has the right (and for a good reason) to do stuff to the text. This may include changing tabs with spaces, word wraps, etc. These are exteremely annoying.
On the other hand, to the best of my knowledge, when I attach a text document, at least when I later view the message, the document is displayed inline (as opposed to actually being inline, which it isn't).
Dimitrie - please find one of my patch submissions, and let me know if it is displayed inline for you. If it is, attaching a file called .diff is the right way to go. If not, attaching .txt may be a solution. I am very much against copying the patch into the message, however.
I have to agree to this position. The only patches that are inconvenient to read for me are the tar files, gzip'ed diff's are no problem.
bye michael
Dimitrie O. Paun wrote:
There is something concerning submitting patches that bothers me to no end: inlining vs. attaching them.
I don't know about others, but for me 99/1 rule applies: I at least skim over 99% of the patches inlines in the message, whereas I bother to read at most 1% of the ones attached/tar.gzed/ziped. Maybe I'm an extreme case, but there's got to be more to it than my personal quirks.
If a patch is sent to wine-patches, it's sent there for peer review. If you don't want the review, send it to Alexandre directly, even though I suggest this is avoided as much as possible. However, it you do send it to wine-patches, please, *please* inline it!
What about a nice patch submission policy: -- unified diff only (required) -- have a decent subject (recommended) -- a long description (optional, if the change warrants it) -- a meaningful ChangeLog entry (required) -- new files, if any, included in patch, diffed against /dev/null (required) -- patch inlined at the end of the message (required) -- one changeset per message
Most of these things are already followed by most people, with the exception of the inlining bit. Alexandre, what about you reject patches that aren't in this format with a pointer to these rules? All this is a matter of habit, and I think we'll all benefit if these rules are followed.
-
Alexandre Julliard -
Andriy Palamarchuk -
David Laight -
Dimitrie O. Paun -
Dimitrie O. Paun -
Duane Clark -
Michael Stefaniuc -
Shachar Shemesh -
Uwe Bonnes