Bug 16003 - Add spelling exclusion patch to QA Test tools
Summary: Add spelling exclusion patch to QA Test tools
Status: CLOSED FIXED
Alias: None
Product: Project Infrastructure
Classification: Unclassified
Component: QA tools (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement
Assignee: Mark Tompsett
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-08 04:52 UTC by Mark Tompsett
Modified: 2016-12-05 21:23 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Add spelling exclusion parameter (1.44 KB, patch)
2016-03-08 04:52 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 16003: Add spelling exclusion switch. (1.37 KB, patch)
2016-03-08 13:13 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 16003 - Add exceptions file logic to codespell call (1.47 KB, patch)
2016-03-08 15:44 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 16003: Add exception pattern list for codespell checks (2.89 KB, patch)
2016-03-09 09:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16003: Add exception pattern list for codespell checks (3.22 KB, patch)
2016-03-09 14:31 UTC, Mark Tompsett
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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.