Date: Sat, 21 Aug 2004 01:23:29 +0300 From: Giorgos Keramidas <keramida@freebsd.org> To: Marc Fonvieille <blackend@freebsd.org> Cc: ceri@freebsd.org Subject: Re: To PR Senders Message-ID: <20040820222329.GA29749@gothmog.gr> In-Reply-To: <20040820205835.GH63041@abigail.blackend.org> References: <20040820133029.GE63041@abigail.blackend.org> <20040820103306.5f0ffb6f.wmoran@potentialtech.com> <20040820205835.GH63041@abigail.blackend.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2004-08-20 22:58, Marc Fonvieille <blackend@freebsd.org> wrote: > On Fri, Aug 20, 2004 at 10:33:06AM -0400, Bill Moran wrote: > > > > I don't have the knowledge to make exact recommendations, but perhaps > > a documented rule of thumb along these lines would help: > > > > "Patches in excess of 200k, or which contain over 50 lines of completely > > new content, should not be attached to the PR, but should be placed on > > an ftp/web server and the URL included. Patches below these limits > > should always be attached to the PR." > > I don't think giving a limit based on lines number is a good idea. > 200k is too much, from my point of view, maybe 50k could be the > maximum... but some may say gzip exists... If you really *do* need a recommendation for a size I think that 50k is about the size that things would probably start getting seriously annoying for dialup users. Yeah, if a size must be explicitly specified 50k is ok. > > I understand that it may be difficult to reach a consensus on exactly > > what limits to set, but I think it would make things a little clearer > > for folks like me. Not necessarily, but if we really have to decide on a size I'd go for the 50k limit that Marc mentioned. Testing on an otherwise idle dialup link I see that transferring 50k needs almost 22 seconds: : pink-> dd if=mbox of=lala bs=50k count=1 : 1+0 records in : 1+0 records out : 51200 bytes transferred in 0.000862 secs (59404804 bytes/sec) : pink-> /usr/bin/time scp lala keramida@igloo.linux.gr: : lala 100% 50KB 50.0KB/s 00:00 : 21.34 real 0.05 user 0.01 sys IMHO, things are going to look real *SLOW* for larger sizes. > I'm waiting for bugmeisters opinion on this point. I think the text leaves the decision to the submitter for a purpose. Very large patches, like for instance a whole new subtree of doc/ with the translation of several articles to ancient Greek, is probably going to be huge to include as a diff. The exact size where linking to an external site makes sense is a bit subjective though. I've seen PRs in the past where people worked on entire chapters of the Handbook and did very well so, by including large text diffs. Common sense should be applied whenever possible, in my opinion. * NOTE: Some other things might be of importance too when deciding if the submitter includes the diff in the PR. For example, there are at least the following two scenarions of people working on PRs: 1) The "offline" hacker. Someone logs into freefall, grabs a set of PRs with `query-pr -F' and saves them to a Unix mailbox file. Then copies the mailbox to his workstation and disconnects from the net. He fires up a mail reader and points it to the PR mailbox. In this case having the patches as part of the PR is MAGNIFICENT. No need to reconnect just to test a single patch! 2) The "online" hacker. Sitting on a workstation at work, Joe Random Hacker fires up a browser and lists the active PRs. He notes down a few PR numbers that he might be interested in and uses his "spare time" between meetings and other random business work to hack merily away. In this case having the URL to the patch isn't really a problem. Both modes of working have their advantages and disadvantages and I've alternated between the two a few times. I'm not sure whether I prefer one or the other though. I'm more inclined towards the first (which would make including the patches pretty much mandatory), but not because of some deeply technical reason. It's just a matter of personal preference.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040820222329.GA29749>