Mike Kupfer <mike.kupfer at sun.com> writes: >> I'd like review for: >> 6740590 Comments check can miss junk preceding bug synopsis >> >> Webrev: >> http://cr.opensolaris.org/~richlowe/6740590 > > I looked at the change to Comments.py; I'll look at the tooltest changes > on Tuesday. > > The change looks fine. > > I also reviewed Comments.py as a whole and noticed a few nits: > > - In comchk(), "blanks" could conceivably overflow. Since we don't > actually do anything with the final count, it'd be better to just set > it to 1 (or True) when we find a blank/empty line.
Agree. > - We're using "^" and "$" as anchors, which can break if a multiline > string ever sneaks in. "\A" and "\Z" would be more robust, I think. I'll check that. > - It'd be helpful to have a comment around line 124 saying what the > r'(\([^)]+\))?$' clause is for. (I figured it out, but it took a > minute.) Yes, that sounds reasonable. > I verified that all the active ARCs have names that match > [A-Z][A-Z]ARC. There was a SARC, but it doesn't have any cases after > 2006. We might need to revisit this check sometime in the future. fiveash had code review comments about that, I'll revisit them. > I think it's safe to assume that there won't be more than 1000 ARC cases > in a year, but we might need to revisit that assumption in a couple > years. In 2005-2007 there were less than 800 cases per year. 2008 > looks to be about the same, I think. I'll look into that, too. I'd prefer not to limit it without reason. > I reviewed all the regular expressions, and I think they're all > appropriately anchored (modulo the issue I noted above about which set > of anchors to use). Thanks, -- Rich
