Skip to content
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.

Sign up for GitHub

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

Attempt workaround of CRAN Windows hang on update.dev.pkg.Rd example #5421

Merged
merged 12 commits into from
Jul 20, 2022
Merged

Attempt workaround of CRAN Windows hang on update.dev.pkg.Rd example #5421

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
28fb214
Make a non empty examples of upgrade pkg
jangorecki Jul 19, 2022
f7fd42e
Merge branch 'master' into jangorecki-patch-1
mattdowle Jul 19, 2022
4ac51c5
temporarily revert change to .Rd to see if we can reproduce CRAN's ha…
mattdowle Jul 19, 2022
de33de6
try --as-cran again to see how long it takes these days, and see if t…
mattdowle Jul 19, 2022
432f4ce
--no-manual again to avoid error 'pdflatex is not available'
mattdowle Jul 19, 2022
8dd63d6
--as-cran comment updated
mattdowle Jul 19, 2022
f0cc8b5
use more standard dontrun{} to save having to comment why packageVers…
mattdowle Jul 19, 2022
419a480
use if(FALSE) instead of dontrun
mattdowle Jul 19, 2022
aee1acf
remove being S3 method of stats::update; but there is now a warning
mattdowle Jul 20, 2022
aebaec9
Highlight in description the great main feature that tests must have …
mattdowle Jul 20, 2022
8eebd1d
update.dev.pkg() renamed update_dev_pkg()
mattdowle Jul 20, 2022
f0b9d8e
man/update.dev.pkg.R filename updated too
mattdowle Jul 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .appveyor.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ environment:
global:
CRAN: http://cloud.r-project.org
WARNINGS_ARE_ERRORS: 1
R_CHECK_ARGS: --no-manual
# R_CHECK_ARGS specified in order to turn off --as-cran (on by default) as that can be slow
R_CHECK_ARGS: --as-cran --no-manual
# --no-manual to avoid error 'pdflatex is not available'
# --as-cran no longer a lot slower (now takes under 6 mins with and without); logs show _R_CHECK_CRAN_INCOMING_=FALSE which could take 5+ mins
_R_CHECK_NO_STOP_ON_TEST_ERROR_: true
# continue tests even if some script failed
_R_CHECK_TESTS_NLINES_: 0
Expand Down
4 changes: 3 additions & 1 deletion man/update.dev.pkg.Rd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@
NULL.
}
\examples{
# data.table::update.dev.pkg()
\dontrun{
data.table::update.dev.pkg()
Copy link
Member Author

@jangorecki jangorecki Jul 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds a risk of some test jobs running with run-dontrun. IMO it is better to completely protect from running it in examples, either by comment or if(FALSE) ...
I don't see a valid use case for running this example. Yet we should still have it there because the usage section is turned into update method for dev.pkg object, which is clearly wrong: #5304 (comment)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running dontrun seems odd to me but ok. I like that code in dontrun is still parsed and checked which isn't the case for comments. So that leads us to if(FALSE) then. Will do.
Regarding the usage section, our .Rd contains \method{update}{dev.pkg} which tells it it's a method. Can't we drop the \method part there, and remove the \alias{update} at the top too?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it looks like a method of update but isn't actually, then that's the sort of thing that is easier to believe leads to a hang somehow.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is _R_CHECK_DONTTEST_EXAMPLES_ but I don't see anything like that for \dontrun

}
}
\seealso{
\code{\link{data.table}}
Expand Down