Hi Forrest,
Great start - thanks. I have a couple of comments...
hdrchk.py:70 - shouldn't there be an else there?
I'm not super keen on doing this SCM-detection dance for every single
file we pass through hdrchk. HdrChk is called by many different
routines/tools, and I hate to add more performance bottlenecks
unnecessarily.
Rich and I were discussing it and we thought it might make sense to
split a new Keywords module, and have the presence-of-keywords detection
moved to there, and only have HdrChk check the position/style of
keywords. This way - Teamware users continue to invoke 'wx keywords'
which calls the old keywords check (which can assume Teamware), and we
invoke Keywords via the Cadmium extension (which can then assume
Mercurial) which saves us having to do the SCM detection.
cheers,
steve
Forrest Wu wrote:
> Hi Rich and Stephen,
>
> I got a webrev of mofication of HdrChk.py in Check module and the
> relevant test cases to support both Teamware and Mercurial.
> What I did:
> 1> Modify interface of function hdrchk to support both Teamware and
> Mercurial.
> 2> Update Checks/HdrChk.py.
> 3> Currently, there is a bug in hdrchk.py. 'hdrchk -a' doesn't work as
> we expected. Fixed it.
> 4> Add a set of header check test examples, m-hdrchk, for mercurial.
> 5> Rename hdrchk to t-hdrchk to keep the test examples for teamware.
> 6> Modify test_hdrchk.py to support both Teamware and Mercurial.
>
> More detail, please see the webrev.
> http://tas.eng/export/pub/Forrest/zfs/scm/scm_stool_1/webrev/
>
> Test results:
> $ python tooltest.py -v HdrChk
> hdrchk with malformed __cplusplus guards ... ok
> hdrchk with missing copyright ... ok
> hdrchk with invalid ending header guard ... ok
> hdrchk with missing header guards ... ok
> hdrchk with relative #include ... ok
> hdrchk with invalid #pragma ident ... ok
> hdrchk leniency with missing __cplusplus guards ... ok
> hdrchk leniency with bad header guard names ... ok
> hdrchk leniency with ident before header guard when lenient or not ... ok
> hdrchk leniency with relative #include ... ok
> hdrchk with missing __cplusplus end guard ... ok
> hdrchk of file not needing __cplusplus guards ... ok
> hdrchk with missing __cplusplus guards ... ok
> hdrchk of valid header ... ok
>
> ----------------------------------------------------------------------
> Ran 14 tests in 0.846s
>
> OK
>
> One issue I run across is I can't put the webrev to cr.grommit.com,
> although I have created a codereview account in it. Any idea?
>
> $ scp -r ./webrev forrest at cr.grommit.com:webrev-HdrChk
> ssh: cr.grommit.com: node name or service name not known
> lost connection
>
> --
> Thanks,
>
> Forrest Wu, Sun China Engineering & Research
> +86 10 62673294 http://blogs.sun.com/forrest
>
--
stephen lau // stevel at sun.com | 650.786.0845 | http://whacked.net
opensolaris // solaris kernel development
