Added spell checker to travis

[dantleech]

• Use sphinxcontrib-spelling
• Requires the enchant library (has to be installed with apt-get)

The list of white-listed words are in spelling_word_list.txt.

The spell checker ignores (at least)

• Code
• Inline text escaped with double-backticks

The whitelist currently contains quite alot of false word.

Next steps:

• Fix all the words which should be escaped in double-backticks, and remove them from the whitelist. f.e. when refering to someMethod. (i.e. someMethod should be escaped)
• Ideally we should also escape the configuration sub-headings which are_configuration_keys. But @wouterj said that you had a policy on that.
• Tidy up and proof read the whitelist :)

[dantleech]

Unfortunately we need to install enchant, so no container based builds :(

[xabbuh]

@dantleech So we can revert this one, right?

[dantleech]

In theory .. I will try

[dantleech]

It should now be possible to do this in containers as enchant has been added to the apt-get whitelist: travis-ci/apt-package-safelist@4138d59

[xabbuh]

@dantleech How long does it take to make that change available on the build systems?

[dantleech]

@xabbuh it should just work. its an undocumented feature at the moment.

[dantleech]

@xabbuh it doesn't seem to work, although the feature is now documented. Asking on IRC.

[xabbuh]

Maybe they need some time to deploy the builds infrastructure. Thank you for investigating.

[dantleech]

Doesn't seem to currently work on forked repositories (but does work on organization pull requests, created an issue:

travis-ci/travis-ci#3199

[xabbuh]

@dantleech I triggered the Travis build and it seems to pass now given that travis-ci/travis-core#429 (except for the intended spell checker failure).

[dantleech]

Yes indeed, seems fine now. However I guess the version of enchant might be different as there are quite a few more spelling errors.

[wouterj]

What is the progress of this PR, @dantleech ?

[dantleech]

Would need to have a look, maybe at the doc day tomorrow.

[OskarStark]

any news on this PR @dantleech

[dantleech]

Will have a look at the next opportunity (some time in the next few days).

[dantleech]

Have had another look and rebased and made some fixes.

I have installed some different spelling backends but the travis installation still seems to be giving 70+ more spelling errors than my local installation. We can add these to the wordlist but before doing further work, is this worth the extra maintenance? Is it likely to be merged? @wouterj @weaverryan

[wouterj]

Personally, I don't think this should be included (or at least, not force a build failure when it fails). It's very usefull, but with all development terms, etc. I think there are too many edge cases to get this correct each time. The last thing I want is people to see a red status on their PRs without them doing anything wrong.

[theofidry]

But this could issue a warning and then the merger also whitelist the unknown word if it was not a spelling mistake.

Another solution would be to be able to do that in command line manually from time to time.

[xabbuh]

What we can think about is doing this only for our branches, but disable the spellchecker in pull requests.

[OskarStark]

But then the maintainers need to fix all the spelling problems which occur and the contributer doesn't know that there is a problem... :(

[javiereguiluz]

After thinking about this proposal, my vote would be 👎

The main reason is that I fear that we'll receive a lot of false positives, so it won't improve much our review process.

[wouterj]

Hi @dantleech! I'm very sorry for the time you put into this, but I'm going to close this pull request.

Having a tool that checks for typos is a very great idea and we would like to have such tool in the docs. However, after quite some testing in this PR, I think we have to conclude that this tool doesn't know developer-speech good enough to be very helpfull.

I hope to see you back in the future with more great doc ideas!

[dantleech]

No problem!

On Tue, Jul 05, 2016 at 03:10:42AM -0700, Wouter J wrote:

Closed [1]#4917.

—
You are receiving this because you were mentioned.
Reply to this email directly, [2]view it on GitHub, or [3]mute the thread.

Reverse link: [4]unknown

References

Visible links

1. Added spell checker to travis #4917
2. Added spell checker to travis #4917 (comment)
3. https://github.com/notifications/unsubscribe/AAgZcaN6P5Uz21-O4r4IzOk9Nay8PBiIks5qSi2igaJpZM4DW7jP
4. Added spell checker to travis #4917 (comment)