Bug 16003

Summary: Add spelling exclusion patch to QA Test tools
Product: Project Infrastructure Reporter: Mark Tompsett <mtompset>
Component: QA toolsAssignee: Mark Tompsett <mtompset>
Status: CLOSED FIXED QA Contact:
Severity: enhancement    
Priority: P5 - low CC: chris, jonathan.druart
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Add spelling exclusion parameter
Bug 16003: Add spelling exclusion switch.
Bug 16003 - Add exceptions file logic to codespell call
Bug 16003: Add exception pattern list for codespell checks
[SIGNED-OFF] Bug 16003: Add exception pattern list for codespell checks

Description Mark Tompsett 2016-03-08 04:52:10 UTC
isnt() is a valid test function, but codespell thinks it is a typo. This adds a -s switch to the qa test tools, to skip the spelling tests.
Comment 1 Mark Tompsett 2016-03-08 04:52:57 UTC Comment hidden (obsolete)
Comment 2 Mark Tompsett 2016-03-08 04:54:41 UTC
It's not really patch 3/3, but I have a couple other tweaks outstanding.
Comment 3 Jonathan Druart 2016-03-08 08:15:23 UTC
use of debug statements, marked as failed QA.

I would not skip this pass, why do you want?
It's certainly better to fix the annoying false positive 'isnt' than completely skip the spelling check.
Comment 4 Mark Tompsett 2016-03-08 13:13:52 UTC
Created attachment 48782 [details] [review]
Bug 16003: Add spelling exclusion switch.

TEST PLAN
---------
Run on a test file with isnt.
-- spelling error
Patch
Run on the same file with a -s
-- spelling tests skipped.
Comment 5 Mark Tompsett 2016-03-08 13:16:58 UTC
(In reply to Jonathan Druart from comment #3)
> use of debug statements, marked as failed QA.

DOH! Valid point.


> I would not skip this pass, why do you want?

Because I rather see SKIPPED than FAILED, when there is a false positive.
Additionally, adding the -s is more typing. Most people won't, like they shouldn't.


> It's certainly better to fix the annoying false positive 'isnt' than
> completely skip the spelling check.

I concede that is the better solution. However, since I don't know how to do that, I did what I know how to do as of this moment.
Comment 6 Mark Tompsett 2016-03-08 15:44:46 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2016-03-09 09:23:44 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2016-03-09 09:25:17 UTC
Mark,
Please have a look at this patch.
I think it does what you want but in a more flexible way: we won't blacklist an entire file, but just a line.
Comment 9 Mark Tompsett 2016-03-09 14:31:30 UTC
Created attachment 48864 [details] [review]
[SIGNED-OFF] Bug 16003: Add exception pattern list for codespell checks

The 'isnt' Test::More subroutine should not be considered as a failure
by the codespell check.
To avoid that this patch introduces the concept of "exception patterns"
for this check.

NOTE: This is better than introducing a '-x {exception file}', because
      it dynamically handles the isnt( issue. If other similar function
      name issues arise in the future, modifications to the
      i_dont_fail_spelling.pl file and the exception list can be made.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 10 Jonathan Druart 2016-03-16 11:29:35 UTC
Pushed, thanks Mark for the signoff.