New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

spell-check package description #784

Merged
merged 7 commits into from May 4, 2015

Conversation

Projects
None yet
3 participants
@krlmlr
Member

krlmlr commented May 3, 2015

Is active only with check(cran = TRUE, check_version = TRUE). Emits a warning if no spell checker is found (checked via utils::aspell(NULL) because the worker function utils:::aspell_find_program is not exported).

R source reference: https://github.com/wch/r-source/blob/deb67e31fd0106dae7ab0d274ab874e56ab1b7b5/src/library/tools/R/QC.R#L6517

@coveralls

This comment has been minimized.

coveralls commented May 3, 2015

Coverage Status

Coverage decreased (-0.1%) to 33.4% when pulling f21079a on krlmlr:aspell into c9470ca on hadley:master.

@coveralls

This comment has been minimized.

coveralls commented May 3, 2015

Coverage Status

Coverage decreased (-0.1%) to 33.4% when pulling f21079a on krlmlr:aspell into c9470ca on hadley:master.

@coveralls

This comment has been minimized.

coveralls commented May 3, 2015

Coverage Status

Coverage decreased (-0.1%) to 33.4% when pulling f21079a on krlmlr:aspell into c9470ca on hadley:master.

Kirill Müller
@coveralls

This comment has been minimized.

coveralls commented May 3, 2015

Coverage Status

Coverage increased (+0.2%) to 33.7% when pulling fe6e9af on krlmlr:aspell into c9470ca on hadley:master.

@hadley

This comment has been minimized.

Member

hadley commented May 4, 2015

Hmmmmmmmm, this doesn't seem like it's going to help a lot of people given that aspell isn't usually installed? (And also maybe it should be used just when cran = TRUE?)

@krlmlr

This comment has been minimized.

Member

krlmlr commented May 4, 2015

I think it's pretty standard on Debian/Ubuntu, at least it's straightforward to install it (via packages aspell and aspell-en, tested on rocker). I guess it can be a pain on Windows or even OS X, that's why I made it optional (with warning).

The spell check is part of the "incoming" check (see R source reference), which is activated only with check_version = TRUE. Only when R-core chooses to split this logic, it will make sense to include it with cran = TRUE.

@hadley

This comment has been minimized.

Member

hadley commented May 4, 2015

Ah, got it. Can you please add a note to NEWS and to the check docs?

Kirill Müller added some commits May 4, 2015

@krlmlr krlmlr changed the title from WIP: spell-check package description to spell-check package description May 4, 2015

@krlmlr

This comment has been minimized.

Member

krlmlr commented May 4, 2015

Updated docs and NEWS. Please document().

@coveralls

This comment has been minimized.

coveralls commented May 4, 2015

Coverage Status

Coverage increased (+0.31%) to 33.81% when pulling 5492497 on krlmlr:aspell into c9470ca on hadley:master.

hadley added a commit that referenced this pull request May 4, 2015

Merge pull request #784 from krlmlr/aspell
spell-check package description

@hadley hadley merged commit 4089358 into r-lib:master May 4, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.31%) to 33.81%
Details
@hadley

This comment has been minimized.

Member

hadley commented May 4, 2015

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment