On Thu, Aug 12, 2010 at 02:25:33PM +0200, Jeremias Maerki wrote: Hi, I have returned and read the discussions. I have the following remarks:
Building fop with jdk 1.4, as required, gives an error when checkstyle-all-5.0.jar is present. The major.minor version 49.0 is not supported. Thus removing checkstyle-4.0 gives an inconsistency between the development environment and the release build environment, which is workable but at the same time annoying. Some methods marked deprecated were part of our (unofficial) API. Deprecation requires some time in which application builders can change over. Indeed, a description of the alternative and a time frame to change over would have been useful. If we remove these methods, we must be prepared to face application builders at our next release. Glenn notes that he used comments to suppress checkstyle warnings in such cases as: - certain uses of package, protected, or public visibility of fields, which would have required a fairly large number of changes to substitute calls to new getX() or setX(...) methods; Leaving the warnings would be a sign that some work is to be done. Suppressing the warnings gives the false idea that no more work is to be done, and that all comments represent a sound judgment to leave the code intentionally as is. Otherwise I am in favour of using warnings to mark code that intentionally does not comply with the rules, at the judgment of the developer. > I would suggest the following as our next steps: > > 1. Clarify the thing with LineBreak*. > 2. Decide (quickly, please) whether to remove the //CS comments or to > allow them for now and optionally do something about them later. (I'm > tending towards removing them but I don't have a problem if we do it the > other way.) > 3. Commit the patch to Trunk more or less as is (pending //CS decision). > 3. Adjust the Checkstyle profile to allow "log" and disallow whitespace > before and after parantheses. Then remove "log"-related //CS constants > and excessive whitespace. > 4. Merge the changes into the Temp_ComplexScripts parts. > 5. Glenn could then provide a new patch against the branch which we > could do a cursory review on. We apply that and experiment with what > he's built. He can continue his work. > 6. We continue to incrementally improve our coding standards. > > I'm happy to do the grunt work. Like Glenn, I don't like to hold > principle discussions right now because that holds up several people > from doing day-to-day work. That doesn't mean we can't hold them, but I > don't see why we have to do it as a precondition to processing this > patch. The patch gets us further but doesn't preclude any futher > improvements later. > > Please, let's get this done. Generally I agree with this plan. Specifically, I do not want to wait for future discussions about better rules. That would make better the enemy of good. I want to take the practical approach that the current work is an improvement over what we had, and must be applied after concensus over the above discussed points. Simon -- Simon Pepping home page: http://www.leverkruid.eu
