Re: [webkit-dev] Changes to prepare-ChangeLog
On Sep 1, 2009, at 12:59 PM, Adam Roben wrote: 2009-09-01 Adam Roben aro...@apple.com Reviewed by NOBODY (OOPS!) Need a short description of this patch (OOPS!) Need a bug title and URL (OOPS!) What do others think? With the detriment of adding Y.A.O. (yet another OOPS!) to the ChangeLog template, I like this. ~Brady -Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
On Thu, 2009-07-09 at 14:32 -0400, tonikitoo (Antonio Gomes) wrote: I grew up listing and seeing people not writing their emails *as it* and publishing on the internet so would replacing m...@apple.com by mjs at apple dot com be a good practice ? I always failed to see much wisdom in this. It might be because I always used highly published email addresses, but then again, I would much rather have something I can just click/copy instead of copy-edit. -- Gustavo Noronha Silva g...@gnome.org GNOME ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
On Fri, 2009-07-10 at 18:18 -0700, John Abd-El-Malek wrote: this changelist. Operations such as gcl upload CHANGENAME (upload to Rietveld) work on the group of files at the same time (also things like gcl diff/commit/revert). The nice thing about it that on such large codebases, developers will often have unrelated changes, and this avoids having to unapply/apply frequently. This looks like a hack that is fixed by using a proper tool, such as git, to me. -- Gustavo Noronha Silva g...@gnome.org GNOME ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
I don't want to open the can of worms that is git vs other source control systems. However given that the instructions for developing WebKit are for svn and there already exists svn-apply/unapply scripts, I think there are enough WebKit devs using svn that warrant improving this process flow. On Wed, Jul 15, 2009 at 12:52 AM, Gustavo Noronha Silva g...@gnome.orgwrote: On Fri, 2009-07-10 at 18:18 -0700, John Abd-El-Malek wrote: this changelist. Operations such as gcl upload CHANGENAME (upload to Rietveld) work on the group of files at the same time (also things like gcl diff/commit/revert). The nice thing about it that on such large codebases, developers will often have unrelated changes, and this avoids having to unapply/apply frequently. This looks like a hack that is fixed by using a proper tool, such as git, to me. -- Gustavo Noronha Silva g...@gnome.org GNOME ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
On Wednesday 15 July 2009 03:59:53 pm John Abd-El-Malek wrote: I don't want to open the can of worms that is git vs other source control systems. However given that the instructions for developing WebKit are for svn and there already exists svn-apply/unapply scripts, I think there are enough WebKit devs using svn that warrant improving this process flow. I think Maciej recently posted a very good action plan for how we can improve the contribution process. In that action plan it was suggested that certain things can be done first (Phase #1 and Phase #2) and then for things that might involve a look at other revision control systems (Phase #3) we can look at those after. This would seem to fall into Phase #3. I think Maciej's was a good action plan. Cheers, Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
To add another tangent to this thread: one thing I don't like about ChangeLog files is that they make it impossible to have multiple concurrent changes in the same checkout. Yes I know that some people use git to get around this, while others use the svn-apply/svn-unapply scripts. But I feel these are just workarounds to get around limitations of the current tools. We should fix the tools instead. If we don't require to change one central file for each change, then bugzilla-tool can be modified to support changelists. On Thu, Jul 9, 2009 at 1:45 PM, Ojan Vafai o...@chromium.org wrote: While having consistency in changelog descriptions is nice, I'm not sure we need to explicitly deal with the case of having multiple authors or multiple bugs for a change. Those are rare enough situations that it's fine for people to include that information however they want. Or, if you don't agree with me, we can at least make those a separate discussion. It would be nice if this discussion could focus on what goes in the default text of a changelog description. The original goal here was to reduce the number of patches that get r-'ed for unnecessary changelog errors. Multiple authors rarely, if ever, results in an r-. Similarly, multiple bugs is rarely an issue for new contributors. Ojan On Thu, Jul 9, 2009 at 1:30 PM, Mark Rowe mr...@apple.com wrote: On 2009-07-09, at 08:02, Joe Mason wrote: Maciej Stachowiak wrote: Now that my attention has been called to it, it's starting to bug me that everyone formats their ChangeLog entries slightly differently. How about this as the canonical format (with prepare-ChangeLog encouraging it)? That reminds me: how do we format a patch with multiple authors? I've been doing this: 2009-07-08 Maciej Stachowiak m...@apple.com Make prepare-ChangeLog less shouty https://bugs.webkit.org/show_bug.cgi?id=27098 Authors: Maciej Stachowiak m...@apple.com, Joe Mason joe.ma...@torchmobile.com Reviewed by Mark Rowe. * Scripts/prepare-ChangeLog: So, the main author (or whichever one is submitting the patch if that's unclear) in the header, then a separate Authors line above the Reviewer line with everyone who deserves credit. I've never seen this format used in WebKit patches. - Mark ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
The workflow I'm looking for is something like svn/perforce changelists. That way I could have multiple unrelated changes coexisting in the same checkout. An example of this is what we use on Chromium: gcl.py (see the latest version at http://src.chromium.org/viewvc/chrome/trunk/tools/depot_tools/gcl.py?view=markup). It allows you to create a changelist and associate a description and a list of files/directory changes with it). The basic usage is that gcl change CHANGENAME pops up a text file in the default editor where a description can be entered. By moving filenames from two sections below it, the developer controls which modified files are in this changelist. Operations such as gcl upload CHANGENAME (upload to Rietveld) work on the group of files at the same time (also things like gcl diff/commit/revert). The nice thing about it that on such large codebases, developers will often have unrelated changes, and this avoids having to unapply/apply frequently. On Fri, Jul 10, 2009 at 5:57 PM, David Kilzer ddkil...@webkit.org wrote: Quilt? http://savannah.nongnu.org/projects/quilt What workflow are you trying to accomplish? I'm not sure I understand what a changelist is in this context and how bugzilla-tool would support them. Dave *From:* John Abd-El-Malek j...@google.com *To:* Ojan Vafai o...@chromium.org *Cc:* WebKit Development webkit-dev@lists.webkit.org; Mark Rowe mr...@apple.com *Sent:* Friday, July 10, 2009 5:11:53 PM *Subject:* Re: [webkit-dev] Changes to prepare-ChangeLog To add another tangent to this thread: one thing I don't like about ChangeLog files is that they make it impossible to have multiple concurrent changes in the same checkout. Yes I know that some people use git to get around this, while others use the svn-apply/svn-unapply scripts. But I feel these are just workarounds to get around limitations of the current tools. We should fix the tools instead. If we don't require to change one central file for each change, then bugzilla-tool can be modified to support changelists. On Thu, Jul 9, 2009 at 1:45 PM, Ojan Vafai o...@chromium.org wrote: While having consistency in changelog descriptions is nice, I'm not sure we need to explicitly deal with the case of having multiple authors or multiple bugs for a change. Those are rare enough situations that it's fine for people to include that information however they want. Or, if you don't agree with me, we can at least make those a separate discussion. It would be nice if this discussion could focus on what goes in the default text of a changelog description. The original goal here was to reduce the number of patches that get r-'ed for unnecessary changelog errors. Multiple authors rarely, if ever, results in an r-. Similarly, multiple bugs is rarely an issue for new contributors. Ojan On Thu, Jul 9, 2009 at 1:30 PM, Mark Rowe mr...@apple.com wrote: On 2009-07-09, at 08:02, Joe Mason wrote: Maciej Stachowiak wrote: Now that my attention has been called to it, it's starting to bug me that everyone formats their ChangeLog entries slightly differently. How about this as the canonical format (with prepare-ChangeLog encouraging it)? That reminds me: how do we format a patch with multiple authors? I've been doing this: 2009-07-08 Maciej Stachowiak m...@apple.com Make prepare-ChangeLog less shouty https://bugs.webkit.org/show_bug.cgi?id=27098 Authors: Maciej Stachowiak m...@apple.com, Joe Mason joe.ma...@torchmobile.com Reviewed by Mark Rowe. * Scripts/prepare-ChangeLog: So, the main author (or whichever one is submitting the patch if that's unclear) in the header, then a separate Authors line above the Reviewer line with everyone who deserves credit. I've never seen this format used in WebKit patches. - Mark ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
Now that my attention has been called to it, it's starting to bug me that everyone formats their ChangeLog entries slightly differently. How about this as the canonical format (with prepare-ChangeLog encouraging it)? 2009-07-08 Maciej Stachowiak m...@apple.com Make prepare-ChangeLog less shouty https://bugs.webkit.org/show_bug.cgi?id=27098 Reviewed by Mark Rowe. * Scripts/prepare-ChangeLog: So specifically: - Reviewed by line would go below, not above, to make git users happy (well, happi*er*) - Bug URL remains below one-line description, for git joy and scannability - No angle brackets around URL, so that you can cut/paste or drag it from a URL field without extra work - No line between URL and summary Is that a format everyone can live with for ChangeLogs and commit messages? If so, I'll post a patch to update prepare-ChangeLog accordingly. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
On Thu, Jul 9, 2009 at 1:32 AM, Maciej Stachowiakm...@apple.com wrote: Is that a format everyone can live with for ChangeLogs and commit messages? If so, I'll post a patch to update prepare-ChangeLog accordingly. LGTM ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
I discussed with Mark Rowe on IRC a bit and it seems like it would be nice if the bug URL could be short enough to just go on one line with the summary. Which turns out to be totally doable. Thus the latest format proposal (the /b/ URL is a redirect): 2009-07-08 Maciej Stachowiak m...@apple.com - http://webkit.org/b/27098 Make prepare-ChangeLog less shouty Reviewed by Mark Rowe. Hypothetical long description goes here. Yeah. Very long and detailed it is. * Scripts/prepare-ChangeLog: On Jul 9, 2009, at 1:32 AM, Maciej Stachowiak wrote: Now that my attention has been called to it, it's starting to bug me that everyone formats their ChangeLog entries slightly differently. How about this as the canonical format (with prepare-ChangeLog encouraging it)? 2009-07-08 Maciej Stachowiak m...@apple.com Make prepare-ChangeLog less shouty https://bugs.webkit.org/show_bug.cgi?id=27098 Reviewed by Mark Rowe. * Scripts/prepare-ChangeLog: So specifically: - Reviewed by line would go below, not above, to make git users happy (well, happi*er*) - Bug URL remains below one-line description, for git joy and scannability - No angle brackets around URL, so that you can cut/paste or drag it from a URL field without extra work - No line between URL and summary Is that a format everyone can live with for ChangeLogs and commit messages? If so, I'll post a patch to update prepare-ChangeLog accordingly. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
2009-07-08 Maciej Stachowiak m...@apple.com - http://webkit.org/b/27098 Make prepare-ChangeLog less shouty Reviewed by Mark Rowe. Hypothetical long description goes here. Yeah. Very long and detailed it is. * Scripts/prepare-ChangeLog: Random nits (since you asked): * Please do not put a - in front of the bug line. What does that buy you besides (sometimes) a bullet in trac.webkit.org? * What does the format look like for bugs with multiple URL links, e.g., to rdar://problem/NNN or to http://crbug.com/N? (The title should not have to be repeated--you should be fixing the same issue for all of them.) * Is the Reviewed by line going to have a blank line above it? (I think it should, but I could be persuaded otherwise.) And from The-World-is-Not-Enough Department: * The commit-log-editor should be smart about condensing duplicate content after the date-name-email lines for each ChangeLog entry, and it should move those lines to the top of the commit message. (This also makes git users happy since the first line would be the bug URL and description, making git log --oneline more useful.) Dave - Original Message From: Maciej Stachowiak m...@apple.com To: Maciej Stachowiak m...@apple.com Cc: WebKit Development webkit-dev@lists.webkit.org Sent: Thursday, July 9, 2009 1:47:16 AM Subject: Re: [webkit-dev] Changes to prepare-ChangeLog I discussed with Mark Rowe on IRC a bit and it seems like it would be nice if the bug URL could be short enough to just go on one line with the summary. Which turns out to be totally doable. Thus the latest format proposal (the /b/ URL is a redirect): 2009-07-08 Maciej Stachowiak - http://webkit.org/b/27098 Make prepare-ChangeLog less shouty Reviewed by Mark Rowe. Hypothetical long description goes here. Yeah. Very long and detailed it is. * Scripts/prepare-ChangeLog: On Jul 9, 2009, at 1:32 AM, Maciej Stachowiak wrote: Now that my attention has been called to it, it's starting to bug me that everyone formats their ChangeLog entries slightly differently. How about this as the canonical format (with prepare-ChangeLog encouraging it)? 2009-07-08 Maciej Stachowiak Make prepare-ChangeLog less shouty https://bugs.webkit.org/show_bug.cgi?id=27098 Reviewed by Mark Rowe. * Scripts/prepare-ChangeLog: So specifically: - Reviewed by line would go below, not above, to make git users happy (well, happi*er*) - Bug URL remains below one-line description, for git joy and scannability - No angle brackets around URL, so that you can cut/paste or drag it from a URL field without extra work - No line between URL and summary Is that a format everyone can live with for ChangeLogs and commit messages? If so, I'll post a patch to update prepare-ChangeLog accordingly. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
Maciej Stachowiak wrote: Now that my attention has been called to it, it's starting to bug me that everyone formats their ChangeLog entries slightly differently. How about this as the canonical format (with prepare-ChangeLog encouraging it)? That reminds me: how do we format a patch with multiple authors? I've been doing this: 2009-07-08 Maciej Stachowiak m...@apple.com Make prepare-ChangeLog less shouty https://bugs.webkit.org/show_bug.cgi?id=27098 Authors: Maciej Stachowiak m...@apple.com, Joe Mason joe.ma...@torchmobile.com Reviewed by Mark Rowe. * Scripts/prepare-ChangeLog: So, the main author (or whichever one is submitting the patch if that's unclear) in the header, then a separate Authors line above the Reviewer line with everyone who deserves credit. Joe ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
On Jul 9, 2009, at 1:47 AM, Maciej Stachowiak wrote: (the /b/ URL is a redirect): I'm like most everything suggested in this thread. But I'm a little sad that these new shorter URLs are redirects. I really like to copy URLs out of the address field, so I’d want the legacy show_bug.cgi one to be the redirect, and the terse one to be the real URL. But that seems impossible, possibly eternally so, if the /b/ one is at webkit.org and not on the actual bugs.webkit.org server. I also don’t love the Authors line as proposed. I normally prefer something a bit less structured and formal for the case where there is some sort of collaboration. See existing log entries with things like Based on idea by etc. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
On Jul 9, 2009, at 8:52 AM, Darin Adler wrote: On Jul 9, 2009, at 1:47 AM, Maciej Stachowiak wrote: (the /b/ URL is a redirect): I'm like most everything suggested in this thread. But I'm a little sad that these new shorter URLs are redirects. I really like to copy URLs out of the address field, so I’d want the legacy show_bug.cgi one to be the redirect, and the terse one to be the real URL. But that seems impossible, possibly eternally so, if the /b/ one is at webkit.org and not on the actual bugs.webkit.org server. Mark and I discussed this and had two ideas: 1) Something draggable/copyable in the bugzilla page that gives you the short URL plus title in ChangeLog format. 2) A keyboard shortcut (perhaps Cmd+Shift+C?) to put the short URL plus title on the pasteboard for ease of pasting I also don’t love the Authors line as proposed. I normally prefer something a bit less structured and formal for the case where there is some sort of collaboration. See existing log entries with things like Based on idea by etc. That's what I usually do too. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
I grew up listing and seeing people not writing their emails *as it* and publishing on the internet so would replacing m...@apple.com by mjs at apple dot com be a good practice ? it reduces the spam count in your inbox for sure. On Thu, Jul 9, 2009 at 2:11 PM, Maciej Stachowiakm...@apple.com wrote: On Jul 9, 2009, at 8:52 AM, Darin Adler wrote: On Jul 9, 2009, at 1:47 AM, Maciej Stachowiak wrote: (the /b/ URL is a redirect): I'm like most everything suggested in this thread. But I'm a little sad that these new shorter URLs are redirects. I really like to copy URLs out of the address field, so I’d want the legacy show_bug.cgi one to be the redirect, and the terse one to be the real URL. But that seems impossible, possibly eternally so, if the /b/ one is at webkit.org and not on the actual bugs.webkit.org server. Mark and I discussed this and had two ideas: 1) Something draggable/copyable in the bugzilla page that gives you the short URL plus title in ChangeLog format. 2) A keyboard shortcut (perhaps Cmd+Shift+C?) to put the short URL plus title on the pasteboard for ease of pasting I also don’t love the Authors line as proposed. I normally prefer something a bit less structured and formal for the case where there is some sort of collaboration. See existing log entries with things like Based on idea by etc. That's what I usually do too. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev -- --Antonio Gomes ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
On Thursday, July 9, 2009 11:06:58 AM, Maciej Stachowiak wrote: On Jul 9, 2009, at 4:33 AM, David Kilzer wrote: * What does the format look like for bugs with multiple URL links, e.g., to rdar://problem/NNN or to http://crbug.com/N? (The title should not have to be repeated--you should be fixing the same issue for all of them.) I could think of two reasonable options? http://webkit.org/b/27098rdar://problem/12345 Make prepare-ChangeLog less shouty Reviewed by Mark Rowe. http://webkit.org/b/27098 Make prepare-ChangeLog less shouty rdar://problem/7123456 Reviewed by Mark Rowe. Preferences? #2 please. * Is the Reviewed by line going to have a blank line above it? (I think it should, but I could be persuaded otherwise.) I don't think it should, if it's one of two special lines at the top. Otherwise the whole ChangeLog entry looks double-spaced and gets harder to read. Yeah, I've noticed that. And from The-World-is-Not-Enough Department: * The commit-log-editor should be smart about condensing duplicate content after the date-name-email lines for each ChangeLog entry, and it should move those lines to the top of the commit message. (This also makes git users happy since the first line would be the bug URL and description, making git log --oneline more useful.) I'm not sure I understand this suggestion. An illustrated example of pulling common lines out of each subsection (with double-spacing): http://trac.webkit.org/changeset/44095 http://trac.webkit.org/changeset/44096 Dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
On 2009-07-09, at 08:02, Joe Mason wrote: Maciej Stachowiak wrote: Now that my attention has been called to it, it's starting to bug me that everyone formats their ChangeLog entries slightly differently. How about this as the canonical format (with prepare- ChangeLog encouraging it)? That reminds me: how do we format a patch with multiple authors? I've been doing this: 2009-07-08 Maciej Stachowiak m...@apple.com Make prepare-ChangeLog less shouty https://bugs.webkit.org/show_bug.cgi?id=27098 Authors: Maciej Stachowiak m...@apple.com, Joe Mason joe.ma...@torchmobile.com Reviewed by Mark Rowe. * Scripts/prepare-ChangeLog: So, the main author (or whichever one is submitting the patch if that's unclear) in the header, then a separate Authors line above the Reviewer line with everyone who deserves credit. I've never seen this format used in WebKit patches. - Mark smime.p7s Description: S/MIME cryptographic signature ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
While having consistency in changelog descriptions is nice, I'm not sure we need to explicitly deal with the case of having multiple authors or multiple bugs for a change. Those are rare enough situations that it's fine for people to include that information however they want. Or, if you don't agree with me, we can at least make those a separate discussion. It would be nice if this discussion could focus on what goes in the default text of a changelog description. The original goal here was to reduce the number of patches that get r-'ed for unnecessary changelog errors. Multiple authors rarely, if ever, results in an r-. Similarly, multiple bugs is rarely an issue for new contributors. Ojan On Thu, Jul 9, 2009 at 1:30 PM, Mark Rowe mr...@apple.com wrote: On 2009-07-09, at 08:02, Joe Mason wrote: Maciej Stachowiak wrote: Now that my attention has been called to it, it's starting to bug me that everyone formats their ChangeLog entries slightly differently. How about this as the canonical format (with prepare-ChangeLog encouraging it)? That reminds me: how do we format a patch with multiple authors? I've been doing this: 2009-07-08 Maciej Stachowiak m...@apple.com Make prepare-ChangeLog less shouty https://bugs.webkit.org/show_bug.cgi?id=27098 Authors: Maciej Stachowiak m...@apple.com, Joe Mason joe.ma...@torchmobile.com Reviewed by Mark Rowe. * Scripts/prepare-ChangeLog: So, the main author (or whichever one is submitting the patch if that's unclear) in the header, then a separate Authors line above the Reviewer line with everyone who deserves credit. I've never seen this format used in WebKit patches. - Mark ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
I didn't solve every possible problem with prepare-ChangeLog, but I tried to make it a bit less shouty. If you don't provide the --bug argument, it includes text like this: - 2009-07-08 Maciej Stachowiak m...@apple.com Reviewed by NOBODY (OOPS!). Need a short description and bug URL (OOPS!) * Scripts/prepare-ChangeLog: - If you do use --bug, it provides text like this: - 2009-07-08 Maciej Stachowiak m...@apple.com Reviewed by NOBODY (OOPS!). Make prepare-ChangeLog less shouty https://bugs.webkit.org/show_bug.cgi?id=27098 * Scripts/prepare-ChangeLog: - It also says this on the console (not in the ChangeLog): -- Please remember to include a detailed description in your ChangeLog entry. -- -- See http://webkit.org/coding/contributing.html for more info -- I think this should address the significant concerns. If there's a desire to reorder the lines or add angle brackets or whatever, people can propose follow-up changes. Also, please review my patch! Regards, Maciej On Jul 7, 2009, at 10:44 AM, Eric Seidel wrote: Oh, I did really like the idea of a prepare-ChangeLog wizard which someone suggested. Where it might ask you some of the relevant questions instead of filling in boilerplate. I continue to find it frustrating that I have to r- patches with bad ChangeLogs. :) I don't think that's so much contributer failure as it is tools failure. The tools should make it easy to do the right thing and hard to post patches which are just gonna get r-'d. -eric On Tue, Jul 7, 2009 at 10:42 AM, Eric Seidele...@webkit.org wrote: I had intended to summarize this long thread which I started. But I've since realized that we're mostly bikeshedding here, so there isn't much actionable takeaway. :( Thank you to all of you for your thoughtful responses! I'm not at all attached to the current YELLING CHANGELOG TEMPLATE. :) But I don't feel like I can draw action from this thread. I'm happy to review further changes to prepare-ChangeLog (as I'm sure others are). So if anyone feels strongly that it's currently broken, please feel free to post a patch. :) -eric On Fri, Jul 3, 2009 at 4:51 PM, David Kilzerddkil...@webkit.org wrote: On Friday, July 3, 2009 2:19:51 PM, Maciej Stachowiak wrote: What I do (and I think many of us do) is use a script that automatically fills in the commit message from the ChangeLog. The script is WebKitTools/Scripts/commit-log-editor. Setting one of these environment variables (depending on whether you're using svn or git) works great (replace $WEBKIT_SRC_DIR with the path to your webkit source, or just set them to commit-log-editor if you've added WebKitTools/Scripts to your PATH): GIT_EDITOR=$WEBKIT_SRC_DIR/WebKitTools/Scripts/commit-log-editor SVN_EDITOR=$WEBKIT_SRC_DIR/WebKitTools/Scripts/commit-log-editor You'll also want to set the EDITOR environment variable unless you use vi to edit your svn/git commit logs. :) Dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
I had intended to summarize this long thread which I started. But I've since realized that we're mostly bikeshedding here, so there isn't much actionable takeaway. :( Thank you to all of you for your thoughtful responses! I'm not at all attached to the current YELLING CHANGELOG TEMPLATE. :) But I don't feel like I can draw action from this thread. I'm happy to review further changes to prepare-ChangeLog (as I'm sure others are). So if anyone feels strongly that it's currently broken, please feel free to post a patch. :) -eric On Fri, Jul 3, 2009 at 4:51 PM, David Kilzerddkil...@webkit.org wrote: On Friday, July 3, 2009 2:19:51 PM, Maciej Stachowiak wrote: What I do (and I think many of us do) is use a script that automatically fills in the commit message from the ChangeLog. The script is WebKitTools/Scripts/commit-log-editor. Setting one of these environment variables (depending on whether you're using svn or git) works great (replace $WEBKIT_SRC_DIR with the path to your webkit source, or just set them to commit-log-editor if you've added WebKitTools/Scripts to your PATH): GIT_EDITOR=$WEBKIT_SRC_DIR/WebKitTools/Scripts/commit-log-editor SVN_EDITOR=$WEBKIT_SRC_DIR/WebKitTools/Scripts/commit-log-editor You'll also want to set the EDITOR environment variable unless you use vi to edit your svn/git commit logs. :) Dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
Oh, I did really like the idea of a prepare-ChangeLog wizard which someone suggested. Where it might ask you some of the relevant questions instead of filling in boilerplate. I continue to find it frustrating that I have to r- patches with bad ChangeLogs. :) I don't think that's so much contributer failure as it is tools failure. The tools should make it easy to do the right thing and hard to post patches which are just gonna get r-'d. -eric On Tue, Jul 7, 2009 at 10:42 AM, Eric Seidele...@webkit.org wrote: I had intended to summarize this long thread which I started. But I've since realized that we're mostly bikeshedding here, so there isn't much actionable takeaway. :( Thank you to all of you for your thoughtful responses! I'm not at all attached to the current YELLING CHANGELOG TEMPLATE. :) But I don't feel like I can draw action from this thread. I'm happy to review further changes to prepare-ChangeLog (as I'm sure others are). So if anyone feels strongly that it's currently broken, please feel free to post a patch. :) -eric On Fri, Jul 3, 2009 at 4:51 PM, David Kilzerddkil...@webkit.org wrote: On Friday, July 3, 2009 2:19:51 PM, Maciej Stachowiak wrote: What I do (and I think many of us do) is use a script that automatically fills in the commit message from the ChangeLog. The script is WebKitTools/Scripts/commit-log-editor. Setting one of these environment variables (depending on whether you're using svn or git) works great (replace $WEBKIT_SRC_DIR with the path to your webkit source, or just set them to commit-log-editor if you've added WebKitTools/Scripts to your PATH): GIT_EDITOR=$WEBKIT_SRC_DIR/WebKitTools/Scripts/commit-log-editor SVN_EDITOR=$WEBKIT_SRC_DIR/WebKitTools/Scripts/commit-log-editor You'll also want to set the EDITOR environment variable unless you use vi to edit your svn/git commit logs. :) Dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
02.07.2009, в 18:05, Adam Roben написал(а): - I prefer having Bug N: before the actual bug description so I don't have to parse the URL myself for the bug number. It also acts as a visual marker when I read the ChangeLog entry. ... Here's an example entry that follows the format that I (and I think Dave) like: +2009-04-20 Adam Roben aro...@apple.com + +Change MemoryStream::createInstance to return a COMPtr + +Part of Bug 25294: All WebKit/win classes should return COMPtrs from +their static constructor members +https://bugs.webkit.org/show_bug.cgi?id=25294 FWIW, I rarely need to know the bug number alone - I need its URL to click or to copy/paste. On the other hand, the suggested format makes it so that one needs to skip over Part of Bug 25294: just to read the bug description, which is not an improvement. - I like putting angle brackets around the URL to set it off visually from other text. Typing those brackets is more work. It's also more difficult to copy/ paste the URL (you could just copy the whole line and paste it into Safari address bar when there were no garbage symbols around the URL) - I generally move the Reviewed by line after the bug number/ description/URL. When you're reading a ChangeLog entry/commit message (especially an older one), it's generally much more interesting which bug is being fixed rather than knowing who reviewed it. (Also, putting the bug description first makes git's one-line description of each commit much more useful than either having a list of dates and the person who wrote the patch or having a list of patch reviewers.) No strong opinion about this one. It seems natural to have the reviewer mentioned close to contributor (which is what we have now). - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
Since this seems to have become the new bikeshed, I'll chime in with my color preference: Reviewed by John Smith (jsm...@webkit.org) https://bugs.webkit.org/show_bug.cgi?id=123456 Fix WebKit being not awesome enough. Make five files more awesome. FWIW, I agree with those who desire to ditch the ChangeLog. WebKit is the only project I've worked on that does such a thing, and I have never gotten any benefit out of it, while I've gotten lots of headache (merge conflicts especially). On Chromium, patches are given a detailed ChangeLog-esque description which is visible in the review tool and becomes the commit log message as well (which links back to the review URL, and is also auto-pasted into the original bug report). This way from any of (bug system, commit logs, review system) you can find information about a particular patch or search for patches matching some comment. This turns out to work quite well in practice. In WebKit I try to give my patches these sorts of comments when I post them for review, but duplicating info between the ChangeLog and the review comments always makes me write less than I otherwise would, and review comments tend to get buried in the sea of noise from bugzilla. PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
On Fri, Jul 3, 2009 at 10:24 AM, Peter Kasting pkast...@google.com wrote: Since this seems to have become the new bikeshed, I'll chime in with my color preference: Reviewed by John Smith (jsm...@webkit.org) https://bugs.webkit.org/show_bug.cgi?id=123456 Fix WebKit being not awesome enough. Make five files more awesome. FWIW, I agree with those who desire to ditch the ChangeLog. WebKit is the only project I've worked on that does such a thing, and I have never gotten any benefit out of it, while I've gotten lots of headache (merge conflicts especially). On Chromium, patches are given a detailed ChangeLog-esque description which is visible in the review tool and becomes the commit log message as well (which links back to the review URL, and is also auto-pasted into the original bug report). This way from any of (bug system, commit logs, review system) you can find information about a particular patch or search for patches matching some comment. This turns out to work quite well in practice. In WebKit I try to give my patches these sorts of comments when I post them for review, but duplicating info between the ChangeLog and the review comments always makes me write less than I otherwise would, and review comments tend to get buried in the sea of noise from bugzilla. I agree that the ChangeLog really is duplicate information and generally a pain to update. I know that there are some people who really like them. Why not fill them in automatically? Just have a tool that once a night/hour/checkin generates entries for the new checkins and puts it somewhere on the web or checks it in. I know one of the concerns was reviews. What I do anyway is copy the ChangeLog description into the optional long description field when posting the patch, why not do that? Maybe it'd even be easy to display that somewhere on the review UI, but otherwise it seems fine to just open the review in another tab in case you need to refer back to it. It seems like the ChangeLog is just a work-around for a lack of tools. And it seems like it wouldn't be too hard to make the tools we've got better. (And yes, I'll even help out if that's the direction everyone chooses to take. :-) J ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
On Jul 2, 2009, at 12:40 AM, Eric Seidel wrote: Agreed. Although, Darin Adler is about the only person I ever see fill in per-file/per-function information. :) Darin is certainly more conscientious about this than most people, but a quick glance at WebCore's ChangeLog shows that many other people also fill in per-file/per-function information. I've found this information invaluable on many occasions when tracking down why a change was made, not just when. John ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
On Jul 3, 2009, at 10:44 AM, Jeremy Orlow wrote: On Fri, Jul 3, 2009 at 10:24 AM, Peter Kasting pkast...@google.com wrote: Since this seems to have become the new bikeshed, I'll chime in with my color preference: Reviewed by John Smith (jsm...@webkit.org) https://bugs.webkit.org/show_bug.cgi?id=123456 Fix WebKit being not awesome enough. Make five files more awesome. FWIW, I agree with those who desire to ditch the ChangeLog. WebKit is the only project I've worked on that does such a thing, and I have never gotten any benefit out of it, while I've gotten lots of headache (merge conflicts especially). On Chromium, patches are given a detailed ChangeLog-esque description which is visible in the review tool and becomes the commit log message as well (which links back to the review URL, and is also auto-pasted into the original bug report). This way from any of (bug system, commit logs, review system) you can find information about a particular patch or search for patches matching some comment. This turns out to work quite well in practice. In WebKit I try to give my patches these sorts of comments when I post them for review, but duplicating info between the ChangeLog and the review comments always makes me write less than I otherwise would, and review comments tend to get buried in the sea of noise from bugzilla. I agree that the ChangeLog really is duplicate information and generally a pain to update. I know that there are some people who really like them. Why not fill them in automatically? Just have a tool that once a night/hour/ checkin generates entries for the new checkins and puts it somewhere on the web or checks it in. What I do (and I think many of us do) is use a script that automatically fills in the commit message from the ChangeLog. I'm not sure why it would be better to copy from the commit messages to the ChangeLog instead of vice versa, or to do it as a separate step instead of at commit time. Are the people who find it a pain to update copying by hand or something? For people who use git, who have presumably already committed their patches locally before posting for review, I think it's fine to do it the other way around and generate the ChangeLog from the commit message when pushing the change back to the master repository. I don't think it's necessary to maintain ChangeLog in your private change branches. I would expect git users to have made tools for dealing with this. For people who use SVN directly, though there's no other place for our tools to pull the log message, so things like bugzilla-tool post-diff could not properly post your diff with a log entry, commit scripts wouldn't be able to fill in the log message that you've already had reviewed, etc. I know one of the concerns was reviews. What I do anyway is copy the ChangeLog description into the optional long description field when posting the patch, why not do that? Maybe it'd even be easy to display that somewhere on the review UI, but otherwise it seems fine to just open the review in another tab in case you need to refer back to it. It seems like the ChangeLog is just a work-around for a lack of tools. And it seems like it wouldn't be too hard to make the tools we've got better. (And yes, I'll even help out if that's the direction everyone chooses to take. :-) ChangeLog is in part just a tradition - all GNU projects do it, and for a while many free software projects were doing it. I personally search ChangeLog for information fairly often and I don't think the commit logs or bug tracker would be equally convenient since they are not available in time-ordered plaintext form. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
On Friday, July 3, 2009 3:20:58 AM, Alexey Proskuryakov wrote: 02.07.2009, в 18:05, Adam Roben написал(а): Here's an example entry that follows the format that I (and I think Dave) like: +2009-04-20 Adam Roben aro...@apple.com + +Change MemoryStream::createInstance to return a COMPtr + +Part of Bug 25294: All WebKit/win classes should return COMPtrs from +their static constructor members +https://bugs.webkit.org/show_bug.cgi?id=25294 FWIW, I rarely need to know the bug number alone - I need its URL to click or to copy/paste. On the other hand, the suggested format makes it so that one needs to skip over Part of Bug 25294: just to read the bug description, which is not an improvement. This probably isn't a typical example since it's a Part N of M bug fix, but the important part is that the change is summarized on the first line (after the date and patch author) to give a quick overview of the patch. - I like putting angle brackets around the URL to set it off visually from other text. Typing those brackets is more work. It's also more difficult to copy/paste the URL (you could just copy the whole line and paste it into Safari address bar when there were no garbage symbols around the URL) Actually, Safari will strip the characters for you if you paste them into the address bar! I didn't know this until a couple of months ago, either. Dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
On Friday, July 3, 2009 2:19:51 PM, Maciej Stachowiak wrote: What I do (and I think many of us do) is use a script that automatically fills in the commit message from the ChangeLog. The script is WebKitTools/Scripts/commit-log-editor. Setting one of these environment variables (depending on whether you're using svn or git) works great (replace $WEBKIT_SRC_DIR with the path to your webkit source, or just set them to commit-log-editor if you've added WebKitTools/Scripts to your PATH): GIT_EDITOR=$WEBKIT_SRC_DIR/WebKitTools/Scripts/commit-log-editor SVN_EDITOR=$WEBKIT_SRC_DIR/WebKitTools/Scripts/commit-log-editor You'll also want to set the EDITOR environment variable unless you use vi to edit your svn/git commit logs. :) Dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
02.07.2009, в 9:44, Eric Seidel написал(а): DETAILED DESCRIPTION OF THE CHANGES GOES HERE. (OOPS!) SEE: http://webkit.org/coding/contributing.html FOR MORE INFORMATION Tests: fast/foo.html * foo.cpp: Added. |n most cases, detailed description of changes would better be per- file or per-function, not in a blob above the list of changes. - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
On Jul 1, 2009, at 10:47 PM, Dan Bernstein wrote: On Jul 1, 2009, at 10:44 PM, Eric Seidel wrote: prepare-ChangeLog should have a --bug= argument and use it for url autofill https://bugs.webkit.org/show_bug.cgi?id=26383 I would much prefer if the bug URL came first. I believe that this is the prevailing style. Looking at the ChangeLog, I see a mix of different styles. I personally prefer the human-readable short description first, and I prefer to prefix it with a dash, like so: - Rename html4.css to html.css, since we target HTML5 now https://bugs.webkit.org/show_bug.cgi?id=26873 I wouldn't mind the URL first if it was much shorter and could be on the same line as the summary. Not too picky about this, but since it's a matter of taste, I thought I'd chime in. - Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
Agreed. Although, Darin Adler is about the only person I ever see fill in per-file/per-function information. :) But you're right, that the message could be made more clear. Maybe something like: CHANGE DESCRIPTION GOES HERE. (OOPS!) SEE: http://webkit.org/coding/contributing.html FOR MORE INFORMATION FILE AND FUNCTION CHANGES SHOULD BE DESCRIBED NEXT TO NAMES BELOW Seems kinda verbose. Hopefully people will actually read http://webkit.org/coding/contributing.html -eric On Thu, Jul 2, 2009 at 12:22 AM, Alexey Proskuryakova...@webkit.org wrote: 02.07.2009, в 9:44, Eric Seidel написал(а): DETAILED DESCRIPTION OF THE CHANGES GOES HERE. (OOPS!) SEE: http://webkit.org/coding/contributing.html FOR MORE INFORMATION Tests: fast/foo.html * foo.cpp: Added. |n most cases, detailed description of changes would better be per-file or per-function, not in a blob above the list of changes. - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
On Thu, Jul 02, 2009 at 01:01:09AM -0700, Adam Barth aba...@webkit.org wrote: On Thu, Jul 2, 2009 at 12:50 AM, Mike Hommeymh+web...@glandium.org wrote: I've always wondered, in the days of atomic commits and advanced SCM, why fill changelogs at all ? Except for CVS, RCS or SCCS, any SCM already stores the log of changes. Keeping a Changelog in the SCM is both a duplication of information and a stick to beat yourself when you cherry-pick or revert changes, or merge branches. When I've ask similar questions in the past, I've been told: 1) Changelogs are easier to search / archive / fix up than commit log messages. For search and archive, nothing prevents you to generate ChangeLogs for that purpose. 2) We can review the Changelog messages using bugzilla's review system, but it's harder to review the commit log message. Not if the patch contains the commit message in its header, like git or mercurial do. Creating a script for svn, if it doesn't already exist, wouldn't be too hard, too. Mike ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
On Wednesday, July 1, 2009 10:44:16 PM, Eric Seidel e...@webkit.org wrote: Results in: 2009-07-01 Eric Seidel e...@webkit.org Reviewed by NOBODY (OOPS!). prepare-ChangeLog should have a --bug= argument and use it for url autofill https://bugs.webkit.org/show_bug.cgi?id=26383 DETAILED DESCRIPTION OF THE CHANGES GOES HERE. (OOPS!) SEE: http://webkit.org/coding/contributing.html FOR MORE INFORMATION Tests: fast/foo.html * foo.cpp: Added. - I prefer having Bug N: before the actual bug description so I don't have to parse the URL myself for the bug number. It also acts as a visual marker when I read the ChangeLog entry. - I like putting angle brackets around the URL to set it off visually from other text. - I generally move the Reviewed by line after the bug number/description/URL. When you're reading a ChangeLog entry/commit message (especially an older one), it's generally much more interesting which bug is being fixed rather than knowing who reviewed it. (Also, putting the bug description first makes git's one-line description of each commit much more useful than either having a list of dates and the person who wrote the patch or having a list of patch reviewers.) Dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
On Jul 2, 2009, at 9:14 AM, David Kilzer wrote: On Wednesday, July 1, 2009 10:44:16 PM, Eric Seidel e...@webkit.org wrote: Results in: 2009-07-01 Eric Seidel e...@webkit.org Reviewed by NOBODY (OOPS!). prepare-ChangeLog should have a --bug= argument and use it for url autofill https://bugs.webkit.org/show_bug.cgi?id=26383 DETAILED DESCRIPTION OF THE CHANGES GOES HERE. (OOPS!) SEE: http://webkit.org/coding/contributing.html FOR MORE INFORMATION Tests: fast/foo.html * foo.cpp: Added. - I prefer having Bug N: before the actual bug description so I don't have to parse the URL myself for the bug number. It also acts as a visual marker when I read the ChangeLog entry. - I like putting angle brackets around the URL to set it off visually from other text. - I generally move the Reviewed by line after the bug number/ description/URL. When you're reading a ChangeLog entry/commit message (especially an older one), it's generally much more interesting which bug is being fixed rather than knowing who reviewed it. (Also, putting the bug description first makes git's one-line description of each commit much more useful than either having a list of dates and the person who wrote the patch or having a list of patch reviewers.) I agree with Dave on all three points. git users gotta stick together. Here's an example entry that follows the format that I (and I think Dave) like: +2009-04-20 Adam Roben aro...@apple.com + +Change MemoryStream::createInstance to return a COMPtr + +Part of Bug 25294: All WebKit/win classes should return COMPtrs from +their static constructor members +https://bugs.webkit.org/show_bug.cgi?id=25294 + +Reviewed by Darin Adler. + +* MemoryStream.cpp: +(MemoryStream::createInstance): Changed to return a COMPtr. +(MemoryStream::Clone): Updated for createInstance change. +* MemoryStream.h: Changed createInstance to return a COMPtr. + +* WebArchive.cpp: +(WebArchive::data): +* WebCoreSupport/EmbeddedWidget.cpp: +(EmbeddedWidget::didReceiveData): +* WebDataSource.cpp: +(WebDataSource::data): +* WebHistory.cpp: +(WebHistory::data): +* WebIconFetcher.cpp: +(WebIconFetcherClient::finishedFetchingIcon): +* WebResource.cpp: +(WebResource::createInstance): +Updated for changes to MemoryStream::createInstance. + -Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
On Jul 2, 2009, at 4:07 AM, Mike Hommey wrote: On Thu, Jul 02, 2009 at 01:01:09AM -0700, Adam Barth aba...@webkit.org wrote: On Thu, Jul 2, 2009 at 12:50 AM, Mike Hommeymh +web...@glandium.org wrote: I've always wondered, in the days of atomic commits and advanced SCM, why fill changelogs at all ? Except for CVS, RCS or SCCS, any SCM already stores the log of changes. Keeping a Changelog in the SCM is both a duplication of information and a stick to beat yourself when you cherry-pick or revert changes, or merge branches. When I've ask similar questions in the past, I've been told: 1) Changelogs are easier to search / archive / fix up than commit log messages. For search and archive, nothing prevents you to generate ChangeLogs for that purpose. 2) We can review the Changelog messages using bugzilla's review system, but it's harder to review the commit log message. Not if the patch contains the commit message in its header, like git or mercurial do. Creating a script for svn, if it doesn't already exist, wouldn't be too hard, too. Indeed, I never manually fill in a changelog and always use the script to autofill it from my git patch. This means that everything in the changelog (at least for me) is a duplicate of what is already in the rcs which is a violation of the DRY principle. If we ever move off svn I will propose that we should no longer maintain the changelog. It is duplicated information and causes update/merge errors/hassle. -Benjamin Meyer ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
Maciej Stachowiak wrote: With SVN at least, it's a lot faster and easier to do a text search on the ChangeLog than to retrieve and search the commit log history. Searching the actual commit logs is very slow online and doesn't work at all offline. The ChangeLog also ends up in static tarball drops, which is useful to people getting those. There are scripts to autogenerate the ChangeLog from the SVN history (I've used svn2cl.pl in the past, don't know if there are better ones). Why not just run one of these when you want to grep the ChangeLog? You could set it up to run automatically when you update if you wanted to always have an up-to-date ChangeLog. It could also be used by the packager to generate ChangeLogs for the tarballs. Joe ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
On Jul 2, 2009, at 12:40 AM, Eric Seidel wrote: Agreed. Although, Darin Adler is about the only person I ever see fill in per-file/per-function information. :) But you're right, that the message could be made more clear. Maybe something like: CHANGE DESCRIPTION GOES HERE. (OOPS!) SEE: http://webkit.org/coding/contributing.html FOR MORE INFORMATION FILE AND FUNCTION CHANGES SHOULD BE DESCRIBED NEXT TO NAMES BELOW Can we change it so that it's not SHOUTING? I'd prefer to see the lines to be edited by the committer start with an indicator, like: --- Change description goes here (OOPS!) --- File and function changes should be described. etc. Simon ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
I also agree and that is the style I use. On Jul 2, 2009, at 7:05 AM, Adam Roben wrote: - I generally move the Reviewed by line after the bug number/ description/URL. When you're reading a ChangeLog entry/commit message (especially an older one), it's generally much more interesting which bug is being fixed rather than knowing who reviewed it. (Also, putting the bug description first makes git's one-line description of each commit much more useful than either having a list of dates and the person who wrote the patch or having a list of patch reviewers.) I agree with Dave on all three points. git users gotta stick together. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
On Jul 2, 2009, at 2:50 PM, Timothy Hatcher wrote: Having it all on the dateline is not the best for git users either. We usually remove that line, since git shows the first line of the commit log in many places. In retrospect, committing to SVN also usually removes this line so I retract the suggestion. On Jul 2, 2009, at 1:44 PM, Maciej Stachowiak wrote: In the future maybe we could consider putting it in the dateline: 2009-06-30 Maciej Stachowiak m...@apple.com revewied by Sam Weinig That way all the blame goes in one place. :-) ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
While clearly a personal preference, I don't like the default prepare-changelog default text and would much rather we revert to the old one. I don't think the new text adds anything and will likely be ignored by most. Can we leave the --bug= option and revert to the old template? -Sam On Wed, Jul 1, 2009 at 10:44 PM, Eric Seidel e...@webkit.org wrote: Background: Reviewers spend too much time correcting errors which should be caught by tools. This is frustrating both to contributers and reviewers. One such class of errors are missing or incorrect information in ChangeLogs. I'm trying to fix that. As part of: http://trac.webkit.org/changeset/45464 prepare-ChangeLog now takes an optional --bug= argument and is able to fill in more than before: % prepare-ChangeLog --bug=26383 Running status to find changed, added, or removed files. Reviewing diff to determine which lines changed. Change author: Eric Seidel e...@webkit.org. Description from bug 26383: prepare-ChangeLog should have a --bug= argument and use it for url autofill. Editing the ../WebCore/ChangeLog file. Results in: 2009-07-01 Eric Seidel e...@webkit.org Reviewed by NOBODY (OOPS!). prepare-ChangeLog should have a --bug= argument and use it for url autofill https://bugs.webkit.org/show_bug.cgi?id=26383 DETAILED DESCRIPTION OF THE CHANGES GOES HERE. (OOPS!) SEE: http://webkit.org/coding/contributing.html FOR MORE INFORMATION Tests: fast/foo.html * foo.cpp: Added. Running prepare-ChangeLog by default will however output more boiler-plate: 2009-07-01 Eric Seidel e...@webkit.org Reviewed by NOBODY (OOPS!). SHORT DESCRIPTION/BUG TITLE GOES HERE (OOPS!) BUG URL GOES HERE (pass --bug= to autofill) DETAILED DESCRIPTION OF THE CHANGES GOES HERE. (OOPS!) SEE: http://webkit.org/coding/contributing.html FOR MORE INFORMATION LIST OF TESTS, OR EXPLANATION WHY TESTING IS IMPOSSIBLE GOES HERE (OOPS!) * foo.cpp: Added. However, hopefully the boilerplate is more helpful than before. Unfortunately everyone will now see: DETAILED DESCRIPTION OF THE CHANGES GOES HERE. (OOPS!) SEE: http://webkit.org/coding/contributing.html FOR MORE INFORMATION in their initial ChangeLogs. This may be annoying to seasoned contributers (and I'm open to removing it). But hopefully it will lead to better ChangeLogs overall. Looking forward to your comments! -eric ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
On Jul 2, 2009, at 4:52 PM, Sam Weinig wrote: While clearly a personal preference, I don't like the default prepare-changelog default text and would much rather we revert to the old one. I don't think the new text adds anything and will likely be ignored by most. Can we leave the --bug= option and revert to the old template? I expressed concern that the new template is too noisy. However, the OOPS part of it can enable the tools to help you notice if you forgot to include a description. Maybe if we got rid of the all-caps other than OOPS, and perhaps prepare-ChangeLog can prompt for a bug number instead of just inserting complaint text by default. - Maciej -Sam On Wed, Jul 1, 2009 at 10:44 PM, Eric Seidel e...@webkit.org wrote: Background: Reviewers spend too much time correcting errors which should be caught by tools. This is frustrating both to contributers and reviewers. One such class of errors are missing or incorrect information in ChangeLogs. I'm trying to fix that. As part of: http://trac.webkit.org/changeset/45464 prepare-ChangeLog now takes an optional --bug= argument and is able to fill in more than before: % prepare-ChangeLog --bug=26383 Running status to find changed, added, or removed files. Reviewing diff to determine which lines changed. Change author: Eric Seidel e...@webkit.org. Description from bug 26383: prepare-ChangeLog should have a --bug= argument and use it for url autofill. Editing the ../WebCore/ChangeLog file. Results in: 2009-07-01 Eric Seidel e...@webkit.org Reviewed by NOBODY (OOPS!). prepare-ChangeLog should have a --bug= argument and use it for url autofill https://bugs.webkit.org/show_bug.cgi?id=26383 DETAILED DESCRIPTION OF THE CHANGES GOES HERE. (OOPS!) SEE: http://webkit.org/coding/contributing.html FOR MORE INFORMATION Tests: fast/foo.html * foo.cpp: Added. Running prepare-ChangeLog by default will however output more boiler- plate: 2009-07-01 Eric Seidel e...@webkit.org Reviewed by NOBODY (OOPS!). SHORT DESCRIPTION/BUG TITLE GOES HERE (OOPS!) BUG URL GOES HERE (pass --bug= to autofill) DETAILED DESCRIPTION OF THE CHANGES GOES HERE. (OOPS!) SEE: http://webkit.org/coding/contributing.html FOR MORE INFORMATION LIST OF TESTS, OR EXPLANATION WHY TESTING IS IMPOSSIBLE GOES HERE (OOPS!) * foo.cpp: Added. However, hopefully the boilerplate is more helpful than before. Unfortunately everyone will now see: DETAILED DESCRIPTION OF THE CHANGES GOES HERE. (OOPS!) SEE: http://webkit.org/coding/contributing.html FOR MORE INFORMATION in their initial ChangeLogs. This may be annoying to seasoned contributers (and I'm open to removing it). But hopefully it will lead to better ChangeLogs overall. Looking forward to your comments! -eric ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Changes to prepare-ChangeLog
Background: Reviewers spend too much time correcting errors which should be caught by tools. This is frustrating both to contributers and reviewers. One such class of errors are missing or incorrect information in ChangeLogs. I'm trying to fix that. As part of: http://trac.webkit.org/changeset/45464 prepare-ChangeLog now takes an optional --bug= argument and is able to fill in more than before: % prepare-ChangeLog --bug=26383 Running status to find changed, added, or removed files. Reviewing diff to determine which lines changed. Change author: Eric Seidel e...@webkit.org. Description from bug 26383: prepare-ChangeLog should have a --bug= argument and use it for url autofill. Editing the ../WebCore/ChangeLog file. Results in: 2009-07-01 Eric Seidel e...@webkit.org Reviewed by NOBODY (OOPS!). prepare-ChangeLog should have a --bug= argument and use it for url autofill https://bugs.webkit.org/show_bug.cgi?id=26383 DETAILED DESCRIPTION OF THE CHANGES GOES HERE. (OOPS!) SEE: http://webkit.org/coding/contributing.html FOR MORE INFORMATION Tests: fast/foo.html * foo.cpp: Added. Running prepare-ChangeLog by default will however output more boiler-plate: 2009-07-01 Eric Seidel e...@webkit.org Reviewed by NOBODY (OOPS!). SHORT DESCRIPTION/BUG TITLE GOES HERE (OOPS!) BUG URL GOES HERE (pass --bug= to autofill) DETAILED DESCRIPTION OF THE CHANGES GOES HERE. (OOPS!) SEE: http://webkit.org/coding/contributing.html FOR MORE INFORMATION LIST OF TESTS, OR EXPLANATION WHY TESTING IS IMPOSSIBLE GOES HERE (OOPS!) * foo.cpp: Added. However, hopefully the boilerplate is more helpful than before. Unfortunately everyone will now see: DETAILED DESCRIPTION OF THE CHANGES GOES HERE. (OOPS!) SEE: http://webkit.org/coding/contributing.html FOR MORE INFORMATION in their initial ChangeLogs. This may be annoying to seasoned contributers (and I'm open to removing it). But hopefully it will lead to better ChangeLogs overall. Looking forward to your comments! -eric ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Changes to prepare-ChangeLog
On Jul 1, 2009, at 10:44 PM, Eric Seidel wrote: prepare-ChangeLog should have a --bug= argument and use it for url autofill https://bugs.webkit.org/show_bug.cgi?id=26383 I would much prefer if the bug URL came first. I believe that this is the prevailing style. Thanks, —Dan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
