Overall, We'd be better off leaving everything as it is.
6.0.24 had a huge amount of changes, and also a series of rapid regressions, and probably more to follow. To keep the branch stable, we should work in a stable manner, RTC has worked well for that

Filip

On 03/08/2010 10:34 AM, 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.
+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.
-1, makes tracing down when and how code changed a pain in the rear. It's not beneficial.
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




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to