As usual this is taking longer than I had hoped, but I am making some good progress. I just (revision 11269) completed the second chunk of source code (all include/*.h files except for qt.h).
On 2010-10-11 15:34-0700 Alan W. Irwin wrote: > Here are the steps I did including all the checks. > > I. A preliminary run of the script gave a runtime error because of > one embedded comment, e.g., > > function( /* this is the first argument*/ x ) > > To my mind this is horrible comment style in any case (and I think > everyone else agrees because there was only one instance). I > therefore edited the file to get rid of the embedded comment and the > check for this issue (anything after the trailing */ on a line > generates a runtime error) will continue to be done by the script. > > N.B. This means new instances of embedded comments will generate > script errors, but I doubt our contributors will generate many (if > any) embedded comments so I haven't worried about implementing a more > graceful way than a run-time error to deal with this case. Actually, in this chunk of C source code I ran into some embedded comments that made sense. So I now leave them unmolested (in the /* ... /* form) in single-line form (i.e., if they are not in the middle of a block comment). If embedded comments do occur by mistake in the middle of a block comment or other comment style shows up which the script doesn't know how to deal with, then scripts/convert_comment.py bails with a runtime error. Furthermore, I have modified scripts/style_source.sh to stop with an overall error message when such runtime errors occur for scripts/convert_comment.py. This new logic worked fine for all files in the latest chunck of source code except for include/plplotcanvas.h which needed some hand editing (revison 11264) to change its pathological comment style into something scripts/convert_comment.py could understand. > > II. Just out of curiosity I checked src for any non-blank characters > in front of // before any changes were applied. > > softw...@raven> grep '[^ ]\+.*//' src/*.[ch] |less > > As expected the small number of instances (mostly URL's inside comments) > were legit. softw...@raven> grep '[^ ]\+.*//' $(ls include/*|grep -v qt.h) |less also showed small number of legit instances of //. > > III. While looking at the potential changes generated by the script > (using "scripts/style_source.sh --diff -au |less") I checked inside > the less command for the following two conditions: > > any trailing blanks; > /. $ > > any insertions that don't have / anywhere on the line (a valid insertion > should always have // on the line so if there is no / anywhere, that > is an error); > > /^[+][^/]+$ > > These searches found nothing. Ditto for second chunk. I also searched in the context diff for any line with a combination of "//" and "*" in it with the following two searches: ///.*\* and /\*.*// A fair number of such patterns turned up, but they all looked like mainstream commentary style (or trailing comment style in the second case) which the python script appears to handle with absolutely no problems. > > IV. I also eyeballed all the potential changes in the ~27000-line > context diff to look for any strange patterns in the resulting // > comments. For various special reasons there were three instances of > indentation errors for the comments that I spotted. (I have noted > these issues and will fix by hand-editing later.) Other than these > minor indentation issues I am completely satisfied with the style of > the commentary now in src. > > convert_comment.py is fairly general and hopefully proof against most > sorts of wacko styles of comments by our contributors. However, there > are no guarantees so the best results will be derived from now on by > not straining this script too much. > > N.B. For example, if you stick with //-style comments from now on, the > script is guaranteed to not attempt to change anything in the files. For the second chunk, the context diff that I eyeballed was "only" ~9100 lines. I didn't find any strange patterns in the resulting comments (after the include/plplotcanvas.h case was handled as discussed above). > > V. The python script is specifically designed not to add or delete any > lines to source files. I checked the number of lines in the files > before and after the changes were applied to confirm this was the > case. Same in this case. > > VI. After "scripts/style_source.sh --apply" was run, I found I had to > run it a second time to take care of some minor realignment that > uncrustify wanted to do because of the comment styling changes. But > subsequent runs showed no further changes were generated. Same in this case. > > VII. I tested the new commentary style by running "cmake" and "make > -j4 test_noninteractive" and "make -j4 test_interactive" in the build > tree. No major build or run-time issues showed up as a result of those > tests. Also, the visual results generated by "make -j4 > test_interactive" looked good (within the general limitation of how > fast they are displayed). Same in this case, but instead of doing those commands by hand, I ran softw...@raven> scripts/comprehensive_test.sh \ --cmake_added_options -DPLD_pdf=ON --build_command 'make -j4' \ --do_nondynamic no --do_static no --do_ctest no \ --do_test_install_tree no --do_test_traditional_install_tree no just to test the extensive header changes in this second chunk as well as to use that updated script in an interesting and useful way. > > N.B. Of course such test results are never complete so keep an eye out > during this release cycle for any run-time bugs that may have been > introduced by this change in commentary style. > > ToDo. Go through the same steps for other chunks of our C code. This second chunk is just a step in the right direction, and there will be more to follow (by removing some of the current temporary limits in scripts/style_source.sh) from e.g., lib, drivers, bindings, and examples, Alan __________________________ Alan W. Irwin Astronomical research affiliation with Department of Physics and Astronomy, University of Victoria (astrowww.phys.uvic.ca). Programming affiliations with the FreeEOS equation-of-state implementation for stellar interiors (freeeos.sf.net); PLplot scientific plotting software package (plplot.org); the libLASi project (unifont.org/lasi); the Loads of Linux Links project (loll.sf.net); and the Linux Brochure Project (lbproject.sf.net). __________________________ Linux-powered Science __________________________ ------------------------------------------------------------------------------ Nokia and AT&T present the 2010 Calling All Innovators-North America contest Create new apps & games for the Nokia N8 for consumers in U.S. and Canada $10 million total in prizes - $4M cash, 500 devices, nearly $6M in marketing Develop with Nokia Qt SDK, Web Runtime, or Java and Publish to Ovi Store http://p.sf.net/sfu/nokia-dev2dev _______________________________________________ Plplot-devel mailing list Plplot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/plplot-devel
