Summary: | Robin Sheat is not a typo | ||
---|---|---|---|
Product: | Project Infrastructure | Reporter: | Mark Tompsett <mtompset> |
Component: | QA tools | Assignee: | Mark Tompsett <mtompset> |
Status: | CLOSED WONTFIX | QA Contact: | |
Severity: | minor | ||
Priority: | P5 - low | CC: | dcook, jonathan.druart, m.de.rooy, magnus, robin |
Version: | unspecified | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | Small patch | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: | Circulation function: | ||
Attachments: |
Bug 21892: Robin Sheat is not a typo [DON'T PUSH]
Bug 21892: Purposeful typo [DO NOT PUSH] Bug 21892: Ignore Sheat when checking spelling Bug 21892: Purposefully update codespell Bug 21892: Add prompt for CodeSpell update |
Description
Mark Tompsett
2018-11-27 17:21:04 UTC
Created attachment 82680 [details] [review] Bug 21892: Robin Sheat is not a typo [DON'T PUSH] This adds a a comment to Koha.pm which should: 1) apply this patch 2) run the qa test tools -- it should fail, because it thinks Sheat is not spelled correctly. Created attachment 82681 [details] [review] Bug 21892: Purposeful typo [DO NOT PUSH] This combined with the former patch gives an intentionally set of words incorrectly spelled. 1) apply the first two patches 2) run qa test tools -- Yes, it should find the bad spellings. Created attachment 82682 [details] [review] Bug 21892: Ignore Sheat when checking spelling 1) without this patch applied to your qa-test-tools run the test plans for the first two patches -- both should fail, with just 1 applied, and both applied to your kohaclone. 2) apply this patch to your qa-test-tools 3) rerun the test plans for the first two patches -- just 1 applied should now pass, but both applied should still fail. Just ignore false messages? We might end up adding all names.. (In reply to Marcel de Rooy from comment #4) > Just ignore false messages? We might end up adding all names.. If there are more names, then we could change it to -I filename. (In reply to Marcel de Rooy from comment #4) > Just ignore false messages? We might end up adding all names.. There aren't cases as special as me. Pushed, note that I did not get the complain with "abbout" (In reply to Jonathan Druart from comment #7) > Pushed, note that I did not get the complain with "abbout" If you look at the default dictionary for codespell, there is a sheat entry. There is no abbout entry. Hence the lack of noise. Tests are failing, -L does not exist in 1.6 (jessie) and 1.8 (stretch) (In reply to Jonathan Druart from comment #9) > Tests are failing, -L does not exist in 1.6 (jessie) and 1.8 (stretch) Even after a: pip install codespell ? (In reply to Jonathan Druart from comment #9) > Tests are failing, -L does not exist in 1.6 (jessie) and 1.8 (stretch) Had similar issues with -L not existing in my base Ubuntu Bionic. sudo apt remove codespell sudo pip uninstall codespell sudo pip install codespell All seems good. (In reply to Jonathan Druart from comment #9) > Tests are failing, -L does not exist in 1.6 (jessie) and 1.8 (stretch) Confirmed that tests are falsely passing spelling. Created attachment 85571 [details] [review] Bug 21892: Purposefully update codespell This detects if codespell has -L option, and if not, it purposefully uninstalls it, and then installs the version it needs via 'pip install'. It also tweaks the detection of codespell installation. Current patch-complexity: Small patch I would not expect a script to remove/install packages without asking anything. I warning message with the detailed steps sounds better to me. What do you think? Created attachment 86572 [details] [review] Bug 21892: Add prompt for CodeSpell update TEST PLAN --------- Now you can say no, get the spelling error. Say yes, and the spelling error goes away. This is just complexifying everything for... what exactly? I am ok with "Sheat" being a typo, really. I am sure Robin will not mind much. I still think that display 3 lines to explain what's need to update to the new version is the best and easiest way to go. Patch reverted. commit db31f7e3380b43ce09798b50ddc5ffd1b17b8836 Revert "Bug 21892: Ignore Sheat when checking spelling" This reverts commit 4e076f26417827494c87677b1d4f3e36ed2f2369. Tests were failing, see also bug 21892 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21892 Looks like the main patch here was pushed, then reverted. If that is correct, the status should probably not be NSO. Resetting to FQA. Feel free to correct me. :-) Too much time spent on this, won't fix. :( (In reply to Marcel de Rooy from comment #4) > Just ignore false messages? We might end up adding all names.. Ignoring false messages sounds reasonable to me. (In reply to Robin Sheat from comment #20) > :( Note that I did not mark it "invalid" ;) (In reply to Jonathan Druart from comment #22) > (In reply to Robin Sheat from comment #20) > > :( > > Note that I did not mark it "invalid" ;) [U+1F602] |