Your vote proposal is not very much readable. Please make a standard single request for vote and check boxes +1, -1 (+0, -0 are irrelevant thought, but might be added as well)
On 03/08/2010 06:34 PM, Konstantin Kolinko wrote:
1. We already have Commit-Then-Review for any documentation, including JavaDoc and code comments. 2. I think that we can have C-T-R for any svn metadata (svn: properties), as those are not the code. I am +1. 3. I think that we can have C-T-R for any whitespace changes in the code. These are generally discouraged, but might be necessary to ease backporting of patches. Note, that whitespace-only changes a) can be verified in viewvc when viewing the committed patch in "Colored Diff" mode. E.g., http://svn.apache.org/viewvc/tomcat/trunk/build.xml?r1=896544&r2=896543&pathrev=896544&diff_format=h b) can be verified by svn diff command: svn diff -x -w will generate a patch ignoring all whitespace changes. Whitespace changes can hinder applying other patches, generated before them. As for line numbers, e.g. when deciphering stack traces, we can always look at the SVN tags that we have from the releases. I am +1 for C-T-R these. 4. Trivial fixes for any English message strings and message constants in the code. I mean, inaccuracies and misprints. I am +1 for C-T-R those. Adding/removing parameters, and changing code that displays these messages is not a trivial change. Things to beware here are single quotes and '{}' brackets. If message string does not have arguments, it is used as is, and those symbols have no special meaning. If it does have arguments, those symbols have special meaning. E.g., there will be an exception if there is '{}' that does not contain a number and is not inside single quotes. 5. Any fixes for any translations. I cannot review the textual part of the changes, only the technical part, and that can be as well done looking at the commit email. The risks here are not very high, as tomcat-i18n-*.jar files do not contain any code in them and can be fixed without recompiling. The technical requirement here is that all *.properties files should contain only 7-bit characters. All others should be \uxxxx escaped. The program to perform such conversion is native2ascii. For consistency, there should be end-of-line character on the last line of the file (as native2ascii always adds it). The specification (JavaDoc for java.util.Properties) says that the files are technically in ISO-8859-1, but, as was discussed around a year ago, we should not allow 8-bit characters from the upper part of ISO-8859-1 charset. The reasons that I remember are: 1). Some IDEs (or IDE plugins) have configuration where by default they are reading *.properties files not in ISO-8859-1, but in the system default encoding. Thus, if the file has character from the upper part of ISO-8859-1, they can be read incorrectly. My own story of observing this was with the PropertiesEditor Plugin for EclipseIDE I suppose that using system encoding by default have some meaning. E.g. when running native2ascii before opening the file in the IDE this setting will allow to open them correctly. 2). We had some files in ISO-8859-1, some in Windows-1252, some in UTF-8. There should have been some reason why that happened. That said, I am +1 for C-T-R for any translation fixes. 6. Should we mark C-T-R commits somehow in the commit message? E.g. writing "C-T-R" or "trivial" in the message? Best regards, Konstantin Kolinko --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
-- ^TM --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
