Re: fixing and maintaining zero reported warnings policy?
This is a heads up -- I will be submitting a patch in a few hours that corrects all outstanding compilation, javadocs, and checkstyle warnings, which together produced about 2200 warning and error messages. This patch changes around 400 files, so it is pervasive. It also incorporates the prior patch I had previously posted for compile warnings only (Bug #49700). I have already obsoleted the patch file attachment there in favor of the one I am preparing to post. I recommend that CTR policy apply in order to merge it into to fop/trunk as quickly as possible in order to not interact with other mods. Note that one compile warning wiill remain (a deprecation) until my recent patch to xmlgraphics-commons is merged and a new build of commons is used with fop builds. G. On Wed, Aug 4, 2010 at 8:20 AM, Glenn Adams gl...@skynav.com wrote: I don't plan to do any refactoring, just fixing warnings. The point of this is to be able to easily find new warnings introduced, and doing so is quite impractical with the current number of extant warnings. If we can get to a state of no warnings, then perhaps we can attempt to maintain that state of affairs for future commits, at least on the trunk. G. On Wed, Aug 4, 2010 at 6:30 AM, spepp...@leverkruid.eu wrote: We have two outstanding branches. All changes you make in this work will, after they have been merged into trunk, be propagated to those branches. I would prefer to restrict the work to the new functionality, and not include other refactoring/correction of warnings. Although I admit that correcting warnings would in itself be very valuable; I guess most warnings were just neglected by their authors, or are in older code. Simon Quoting Glenn Adams gl...@skynav.com: Would anyone mind if I submit a patch that fixes all the outstanding warnings, etc., reported during the build process and by checkstyles and findbugs on the trunk? More importantly, if I do this, is it possible to adhere to a zero tolerance policy on warnings for future commits? I find the 3000 or so warnings currently produced to be a rather significant impediment to doing work on this code base, or at least, in preventing an avalanche of new warnings upon future commits, given the trouble required to determine the diffs between new warnings and old warnings. Perhaps this isn't a problem for changes to one file, but for changes to a hundred files, it is a major headache. Anyway, some of these 3000 are actually real, lurking bugs. I'm willing to do the cleanup work if others will help maintain cleanliness going forward. Regards, Glenn This message was sent using IMP, the Internet Messaging Program.
Re: fixing and maintaining zero reported warnings policy?
Hi Glenn, (Moving to general@ as maybe this is something we want to do at the XML Graphics project level. Please continue discussion there.) Thanks for bringing up this topic. I personally agree that a zero-warning policy would be A Good Thing. In theory newly committed code should have no Checkstyle warning, but I’m not sure that policy is thoroughly followed. Before enforcing such a policy it is necessary to come up with a Checkstyle file on which everyone agrees. The current one is not properly customized IMO. I started to create a new one from scratch a long time ago but never got round to finishing and testing it. Feel free to submit such a file. Once everyone is happy with it then you can start removing all the warnings on the current code if you feel like doing it. But doing it now would be a bit premature. I can’t really comment on findbugs, I must admit that I’ve never used it (me blushing with shame). This would probably also be a good thing to enforce its usage, but I suppose it also needs some customization. Thanks, Vincent Glenn Adams wrote: Would anyone mind if I submit a patch that fixes all the outstanding warnings, etc., reported during the build process and by checkstyles and findbugs on the trunk? More importantly, if I do this, is it possible to adhere to a zero tolerance policy on warnings for future commits? I find the 3000 or so warnings currently produced to be a rather significant impediment to doing work on this code base, or at least, in preventing an avalanche of new warnings upon future commits, given the trouble required to determine the diffs between new warnings and old warnings. Perhaps this isn't a problem for changes to one file, but for changes to a hundred files, it is a major headache. Anyway, some of these 3000 are actually real, lurking bugs. I'm willing to do the cleanup work if others will help maintain cleanliness going forward. Regards, Glenn
RE: fixing and maintaining zero reported warnings policy?
I've wondered about that myself. I tried working with the Trunk and got countless warnings. They included dead code, deprecated methods, and unused assigned variables among other things. To sift through you could go into project specific compile options in Eclipse and tell it to ignore them if you have the project created as source. There may be an option to ignore on the ant build that's not as obvious but clean code would be appreciated, even as much as I appreciate the dirty code if the warnings don't crash anything and it got some useful features implemented faster. I would hope there's less urgency to implement now that the project has passed the 1.0 mark and code can be kept clean. From: Glenn Adams [mailto:gl...@skynav.com] Sent: Tuesday, August 03, 2010 4:27 AM To: FOP Developers Subject: fixing and maintaining zero reported warnings policy? Would anyone mind if I submit a patch that fixes all the outstanding warnings, etc., reported during the build process and by checkstyles and findbugs on the trunk? More importantly, if I do this, is it possible to adhere to a zero tolerance policy on warnings for future commits? I find the 3000 or so warnings currently produced to be a rather significant impediment to doing work on this code base, or at least, in preventing an avalanche of new warnings upon future commits, given the trouble required to determine the diffs between new warnings and old warnings. Perhaps this isn't a problem for changes to one file, but for changes to a hundred files, it is a major headache. Anyway, some of these 3000 are actually real, lurking bugs. I'm willing to do the cleanup work if others will help maintain cleanliness going forward. Regards, Glenn
Re: fixing and maintaining zero reported warnings policy?
it may be difficult to agree on a particular checkstyles policy, and I really don't want to force creating or agreeing on such a policy, e.g., i don't like to break lines for some arbitrary limit and i use nested blocks and inline assignment, while others may not like that; however, i try to respect the style of the author of a given file, so if the author uses a line length limit, i will keep to it as well as other styles; for me, it is more important to create and maintain a zero warning policy; i don't care if the warnings are suppressed as long as an author gave them legitimate review and decided they were acceptable; both checkstyle and findbugs provides mechanisms for filtering the output to suppress warnings, and that is what i would prefer using, to permit what i call blessed warnings, etc. the important thing is that some author/contributor has taken the effort to review warnings and bless them if they are to be allowed or to remove their underlying cause if not; that is what i propose to do and to maintain; G. On Tue, Aug 3, 2010 at 6:45 PM, Vincent Hennebert vhenneb...@gmail.comwrote: Hi Glenn, (Moving to general@ as maybe this is something we want to do at the XML Graphics project level. Please continue discussion there.) Thanks for bringing up this topic. I personally agree that a zero-warning policy would be A Good Thing. In theory newly committed code should have no Checkstyle warning, but I’m not sure that policy is thoroughly followed. Before enforcing such a policy it is necessary to come up with a Checkstyle file on which everyone agrees. The current one is not properly customized IMO. I started to create a new one from scratch a long time ago but never got round to finishing and testing it. Feel free to submit such a file. Once everyone is happy with it then you can start removing all the warnings on the current code if you feel like doing it. But doing it now would be a bit premature. I can’t really comment on findbugs, I must admit that I’ve never used it (me blushing with shame). This would probably also be a good thing to enforce its usage, but I suppose it also needs some customization. Thanks, Vincent Glenn Adams wrote: Would anyone mind if I submit a patch that fixes all the outstanding warnings, etc., reported during the build process and by checkstyles and findbugs on the trunk? More importantly, if I do this, is it possible to adhere to a zero tolerance policy on warnings for future commits? I find the 3000 or so warnings currently produced to be a rather significant impediment to doing work on this code base, or at least, in preventing an avalanche of new warnings upon future commits, given the trouble required to determine the diffs between new warnings and old warnings. Perhaps this isn't a problem for changes to one file, but for changes to a hundred files, it is a major headache. Anyway, some of these 3000 are actually real, lurking bugs. I'm willing to do the cleanup work if others will help maintain cleanliness going forward. Regards, Glenn
Re: fixing and maintaining zero reported warnings policy?
It seems to me the following: Glenn, perhaps you could submit a separate de-warning patch or patches against trunk. That could be reviewed, applied, and downmerged into the complex script work. That might make this more manageable to the committers. I would also respectfully wonder if diffing the build output before and after your patches might be sufficient for your purposes. On Tue, Aug 3, 2010 at 8:20 PM, Glenn Adams gl...@skynav.com wrote: I don't plan to do any refactoring, just fixing warnings. The point of this is to be able to easily find new warnings introduced, and doing so is quite impractical with the current number of extant warnings. If we can get to a state of no warnings, then perhaps we can attempt to maintain that state of affairs for future commits, at least on the trunk. G. On Wed, Aug 4, 2010 at 6:30 AM, spepp...@leverkruid.eu wrote: We have two outstanding branches. All changes you make in this work will, after they have been merged into trunk, be propagated to those branches. I would prefer to restrict the work to the new functionality, and not include other refactoring/correction of warnings. Although I admit that correcting warnings would in itself be very valuable; I guess most warnings were just neglected by their authors, or are in older code. Simon Quoting Glenn Adams gl...@skynav.com: Would anyone mind if I submit a patch that fixes all the outstanding warnings, etc., reported during the build process and by checkstyles and findbugs on the trunk? More importantly, if I do this, is it possible to adhere to a zero tolerance policy on warnings for future commits? I find the 3000 or so warnings currently produced to be a rather significant impediment to doing work on this code base, or at least, in preventing an avalanche of new warnings upon future commits, given the trouble required to determine the diffs between new warnings and old warnings. Perhaps this isn't a problem for changes to one file, but for changes to a hundred files, it is a major headache. Anyway, some of these 3000 are actually real, lurking bugs. I'm willing to do the cleanup work if others will help maintain cleanliness going forward. Regards, Glenn This message was sent using IMP, the Internet Messaging Program.
