[chromium-dev] Re: Reviewing commit messages
On Tue, Feb 3, 2009 at 5:26 AM, Dean McNamee de...@chromium.org wrote: Ever since I've been using git, I have super great and quick access to our full history and commit logs. Being in a different time zone, I usually warm up in the morning by reading yesterday's commits. I've found in general we are very good and strict about review code, but hardly anyone ever comments on the review description. I often have to follow the code review link in the commit, and read through 30 review messages to get the information I wanted. A few problems I see very commonly: - Simple style issues: Typos / spelling errors. Wacky line breaks, this is partially rietveld's fault, but we should standardize on something and fix rietveld / gcl. - Bigger issues A review description that should be a review message. Commit 1234: Make this change. Please review carefully, what do you think about the last half? A funny commit message. I'm not all dry, I like jokes, but they're not that funny in 1 year when you're trying to find a regression. A review description that is completely stale from the code. When we iterate on rounds of feedback, we should also comment on the commit description to make sure it's updated. Because of the way GCL works, we do the commit description first, and not last. There can be a huge shift in the code between those points, and often the commit message goes forgotten. So, here is my proposal. We review commit descriptions along with code reviews. In general, we should think of every commit description as if it were a comment in the code. That means proper spelling, formatting, up to date, etc. It's important to remember that although you might be making a quick fix that is obvious to everyone in the room (hey, the tree was red!), in 2 days no one will remember that situation. The code description will be left as 'qucik fix!', when it instead should have been Correct a typo in XXX, the previous code fails to compile. Are you with me? Thanks -- dean Great suggestion. (I really wish Reitveld would be OK with 80 column-wide comments!) At least our revision history has the review links. Those are invaluable. -Darin --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Reviewing commit messages
Dean, I agree with all your suggestions. Re: typos/spelling errors: I recently started to fix typos/spelling errors in the description of a CL I'm reviewing by clicking Edit Issue. (Rietveld allows a reviewer to edit the description of a CL.) Wan-Teh --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] chromium linux meeting note
news flash: the team has decided to make the linux port more like the mac port, i.e. it will use a Views-like gtk layer rather than porting Views. More details later from somebody who knows more... --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] [Extensions] List of possible APIs
I've created a list of possible extension APIs to the wiki, just so that we have it: http://dev.chromium.org/developers/design-documents/extensions/apis We haven't yet prioritized these (hence the empty priority column). We'll need to figure that out next, probably based on number and popularity of extensions each would enable. If you see something missing, feel free to either edit that page or reply here. - a --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Reviewing commit messages
On Tue, Feb 3, 2009 at 8:53 AM, Wan-Teh Chang w...@google.com wrote: Re: typos/spelling errors: I recently started to fix typos/spelling errors in the description of a CL I'm reviewing by clicking Edit Issue. (Rietveld allows a reviewer to edit the description of a CL.) Actually, I think it's a custom hack I made to rietveld -- people with @chromium.org email addresses can edit any aspect of an issue on our site. --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] build problem (chromium.r9040.tar.gz)
I have downloaded r9040 tar-ball from the chromium web site and built it in VS2005 (Before building it, I have removed the previous version completely in my hard disk.) But it made lots of errors. - One of those was 'generated_resources.h'. So, I have looked into it in detail and found that the batch file 'grit_resource_file.bat' is a little weird. It has following lines in it. - :: Put cygwin in the path call %SolutionDir%\..\third_party\cygwin\setup_env.bat %SolutionDir%\..\third_party\python_24\python.exe %SolutionDir%\.. \tools\grit\grit.py -i %InFile% build -o %OutDir% - It contains 'third_party\cygwin' and 'third_party\python_24'. But as far as I know, those are not available anymore. (r9040 doesn't have 'cygwin' or 'python_24' inside the 'third_party' folder.) How do you think about this? And what should I do for building the project successfully? --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] using string16
[A bunch of the team met up today to hammer out some decisions.] In brief: for strings that are known to be Unicode (that is, not random byte strings read from a file), we will migrate towards using string16. This means all places we use wstring should be split into the appropriate types: - byte strings should be string or vectors of chars - paths should be FilePath - urls should be GURL - UI strings, etc. should be string16. string16 uses UTF-16 underneath. It's equivalent to wstring on Windows, but wstring involves 4-byte characters on Linux/Mac. Some important factors were: - we don't have too many strings in this category (with the huge exception of WebKit), so memory usage isn't much of an issue - it's the native string type of Windows, Mac, and WebKit - we want it to be explicit (i.e. a compile error) when you accidentally use a byte string in a place where we should know the encoding (which std::string and UTF-8 doesn't allow) - we still use UTF-8 in some places (like the history full-text database) where space is more of a concern --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: build problem (chromium.r9040.tar.gz)
You must use gclient sync to check out. What is the URL for the build instructions were you following? I can fix them. On Tue, Feb 3, 2009 at 6:04 PM, DeArto20 sy3...@gmail.com wrote: I have downloaded r9040 tar-ball from the chromium web site and built it in VS2005 (Before building it, I have removed the previous version completely in my hard disk.) But it made lots of errors. - One of those was 'generated_resources.h'. So, I have looked into it in detail and found that the batch file 'grit_resource_file.bat' is a little weird. It has following lines in it. - :: Put cygwin in the path call %SolutionDir%\..\third_party\cygwin\setup_env.bat %SolutionDir%\..\third_party\python_24\python.exe %SolutionDir%\.. \tools\grit\grit.py -i %InFile% build -o %OutDir% - It contains 'third_party\cygwin' and 'third_party\python_24'. But as far as I know, those are not available anymore. (r9040 doesn't have 'cygwin' or 'python_24' inside the 'third_party' folder.) How do you think about this? And what should I do for building the project successfully? --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: using string16
An angel loses its wings for each 00 byte in UTF-16. Is 'host' measured in base-2 or base-10? Linus On Tue, Feb 3, 2009 at 6:11 PM, Evan Martin e...@chromium.org wrote: [A bunch of the team met up today to hammer out some decisions.] In brief: for strings that are known to be Unicode (that is, not random byte strings read from a file), we will migrate towards using string16. This means all places we use wstring should be split into the appropriate types: - byte strings should be string or vectors of chars - paths should be FilePath - urls should be GURL - UI strings, etc. should be string16. string16 uses UTF-16 underneath. It's equivalent to wstring on Windows, but wstring involves 4-byte characters on Linux/Mac. Some important factors were: - we don't have too many strings in this category (with the huge exception of WebKit), so memory usage isn't much of an issue - it's the native string type of Windows, Mac, and WebKit - we want it to be explicit (i.e. a compile error) when you accidentally use a byte string in a place where we should know the encoding (which std::string and UTF-8 doesn't allow) - we still use UTF-8 in some places (like the history full-text database) where space is more of a concern --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: build problem (chromium.r9040.tar.gz)
I have not checked out the code using 'gclient sync'. Anyway, The URL for the build instructions I followed. - 1. http://dev.chromium.org/developers/how-tos/build-instructions-windows Link to here - 2. http://dev.chromium.org/developers/how-tos/get-the-code Link to here - 3. http://build.chromium.org/buildbot/archives/chromium_tarball.html And it started to download chromium.r9045.tar.gz automatically. --- I visited the server (http://src.chromium.org/viewvc/chrome/trunk/ src/...) and looked over the latest version (directory revision 9125) of the grit_resource_file.bat and third_party folder. And I verified that there are no differences between them (the latest) and my local files. - There are not python_24 and cygwin under third_party folder and grit_resource_file.bat still references them. So, in this case, if I check out the latest files using 'gclient sync', is there anything that is changed? On 2월4일, 오전11시12분, Evan Martin e...@chromium.org wrote: You must use gclient sync to check out. What is the URL for the build instructions were you following? I can fix them. On Tue, Feb 3, 2009 at 6:04 PM, DeArto20 sy3...@gmail.com wrote: I have downloaded r9040 tar-ball from the chromium web site and built it in VS2005 (Before building it, I have removed the previous version completely in my hard disk.) But it made lots of errors. - One of those was 'generated_resources.h'. So, I have looked into it in detail and found that the batch file 'grit_resource_file.bat' is a little weird. It has following lines in it. --- --- --- :: Put cygwin in the path call %SolutionDir%\..\third_party\cygwin\setup_env.bat %SolutionDir%\..\third_party\python_24\python.exe %SolutionDir%\.. \tools\grit\grit.py -i %InFile% build -o %OutDir% --- --- --- It contains 'third_party\cygwin' and 'third_party\python_24'. But as far as I know, those are not available anymore. (r9040 doesn't have 'cygwin' or 'python_24' inside the 'third_party' folder.) How do you think about this? And what should I do for building the project successfully? --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: build problem (chromium.r9040.tar.gz)
What evan means is that after downloading the tar ball, you need to run gclient sync to get all the platform specific dependencies. We recently started generating the source tar ball on a regular basis and it doesn't include all the Windows and Mac dependencies from the src/third_party directory. Running gclient sync will download these platform specific dependencies for you. On Tue, 3 Feb 2009, DeArto20 wrote: I have not checked out the code using 'gclient sync'. Anyway, The URL for the build instructions I followed. - 1. http://dev.chromium.org/developers/how-tos/build-instructions-windows Link to here - 2. http://dev.chromium.org/developers/how-tos/get-the-code Link to here - 3. http://build.chromium.org/buildbot/archives/chromium_tarball.html And it started to download chromium.r9045.tar.gz automatically. --- I visited the server (http://src.chromium.org/viewvc/chrome/trunk/ src/...) and looked over the latest version (directory revision 9125) of the grit_resource_file.bat and third_party folder. And I verified that there are no differences between them (the latest) and my local files. - There are not python_24 and cygwin under third_party folder and grit_resource_file.bat still references them. So, in this case, if I check out the latest files using 'gclient sync', is there anything that is changed? On 2¿ù4ÀÏ, ¿ÀÀü11½Ã12ºÐ, Evan Martin e...@chromium.org wrote: You must use gclient sync to check out. What is the URL for the build instructions were you following? I can fix them. On Tue, Feb 3, 2009 at 6:04 PM, DeArto20 sy3...@gmail.com wrote: I have downloaded r9040 tar-ball from the chromium web site and built it in VS2005 (Before building it, I have removed the previous version completely in my hard disk.) But it made lots of errors. - One of those was 'generated_resources.h'. So, I have looked into it in detail and found that the batch file 'grit_resource_file.bat' is a little weird. It has following lines in it. --- --- --- :: Put cygwin in the path call %SolutionDir%\..\third_party\cygwin\setup_env.bat %SolutionDir%\..\third_party\python_24\python.exe %SolutionDir%\.. \tools\grit\grit.py -i %InFile% build -o %OutDir% --- --- --- It contains 'third_party\cygwin' and 'third_party\python_24'. But as far as I know, those are not available anymore. (r9040 doesn't have 'cygwin' or 'python_24' inside the 'third_party' folder.) How do you think about this? And what should I do for building the project successfully? --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: build problem (chromium.r9040.tar.gz)
On Tue, Feb 3, 2009 at 9:05 PM, t...@chromium.org wrote: What evan means is that after downloading the tar ball, you need to run gclient sync to get all the platform specific dependencies. We recently started generating the source tar ball on a regular basis and it doesn't include all the Windows and Mac dependencies from the src/third_party directory. Running gclient sync will download these platform specific dependencies for you. In that case, the build instructions are out of date. I updated the getting-the-code page to reflect that this is now required. Brett --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: build problem (chromium.r9040.tar.gz)
On Tue, Feb 3, 2009 at 9:55 PM, Brett Wilson bre...@chromium.org wrote: On Tue, Feb 3, 2009 at 9:05 PM, t...@chromium.org wrote: What evan means is that after downloading the tar ball, you need to run gclient sync to get all the platform specific dependencies. We recently started generating the source tar ball on a regular basis and it doesn't include all the Windows and Mac dependencies from the src/third_party directory. Running gclient sync will download these platform specific dependencies for you. In that case, the build instructions are out of date. I updated the getting-the-code page to reflect that this is now required. If this is now required, we screwed up somewhere. The goal of the tarball is mainly to give an easy way for people to download and build chromium. If they need to call gclient sync, it defeats the purpose. Are you sure we really need that? Nicolas Brett --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: build problem (chromium.r9040.tar.gz)
I thought the point of the tarball was to bootstrap the process, not to remove the gclient dependencies. Doing a full checkout via gclient can be slow (lots of requests for lots of files) and taxing on our server (same reason). Having some base from which to start speeds it up and reduces load on the server. I don't think we ever had a hard requirement that the tarball be everything one would ever need, it just happened that the first versions did happen to be so. -Ian On Tue, Feb 3, 2009 at 9:57 PM, Nicolas Sylvain nsylv...@chromium.orgwrote: On Tue, Feb 3, 2009 at 9:55 PM, Brett Wilson bre...@chromium.org wrote: On Tue, Feb 3, 2009 at 9:05 PM, t...@chromium.org wrote: What evan means is that after downloading the tar ball, you need to run gclient sync to get all the platform specific dependencies. We recently started generating the source tar ball on a regular basis and it doesn't include all the Windows and Mac dependencies from the src/third_party directory. Running gclient sync will download these platform specific dependencies for you. In that case, the build instructions are out of date. I updated the getting-the-code page to reflect that this is now required. If this is now required, we screwed up somewhere. The goal of the tarball is mainly to give an easy way for people to download and build chromium. If they need to call gclient sync, it defeats the purpose. Are you sure we really need that? Nicolas Brett --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
