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.

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

Include travis folding markers #1654

Closed
hadley opened this issue Jan 7, 2019 · 12 comments
Closed

Include travis folding markers #1654

hadley opened this issue Jan 7, 2019 · 12 comments
Milestone

Comments

@hadley
Copy link
Contributor

hadley commented Jan 7, 2019

When building large or many rmarkdown files on travis (e.g. https://travis-ci.org/hadley/adv-r/jobs/476364254), it is hard to find problems because the log file is thousands of lines long. Travis provides a way to handle this through folding markers (these are undocumented but used by all build systems so can not change).

I think it would be very useful if knitr detected that it was being called in travis (by checking the TRAVIS env var), and inserting the folding comments at the start and end of each document.

@cderv
Copy link
Collaborator

cderv commented Jan 19, 2019

There is in fact some helper function in another package from @yihui called crandalf. There is travis_fold() already there. This is a good documentation of this feature.
I put this here for reference while trying to work on that.

@cderv
Copy link
Collaborator

cderv commented Jan 22, 2019

There is now a PR where this is working but folding is very slow when travis rendered it.
knitr prints a lots of things with the progress bar and all, and there may be a way to reduce this. It also depends on what should be log at the end in a travis rendering.

I was wondering if it could not be better to deactivate some logging, to show only what it is interesting. There is the quiet argument that completely deactivate printing. not sure if warning or error are passed. knitr's option progress maybe reduce less.

Does the slow JS rendering is an issue ? I find it very annoying when loading the page.

I also found out that travis a 10000 max lines policy and if above it will not print more and one should use raw log. Folding does not reduce this count. So, for a big book with bookdown, with all the printing, 10000 could be reached. Right now it is 9000+. Reducing the log output could be interesting.

@hadley
Copy link
Contributor Author

hadley commented Jan 22, 2019

Yeah, it looks like the travis folding is now where near as useful as I'd hoped — it's better with the folding than without, but it's not a terribly big improvement. I agree that we'll need greater changes to the logging to make it more effective. In an ideal world, I think we'd only log when something goes wrong, but that's likely to require much greater changes to knitr.

@cderv
Copy link
Collaborator

cderv commented Jan 27, 2019

I recalled that knitr can write ouput message, error and warning to R console instead of the document, with the correct options set. In addition of quiet = TRUE, this is a way to have a small log with messages, warnings and errors in the log output, and not in the document. (see example). In this case the folding is not really needed or at least not in the correct place. (message, error, and warnings are not inside the folds).

Is there something else you would like to consult in bookdown rendering log ouput ?

I tried to deactivate only the progress bar, but I did not manage that. Seems like progress knitr option is not working for that.

@hadley
Copy link
Contributor Author

hadley commented Jan 28, 2019

That looks like a big improvement! Is it possible to get the warnings inside the folds too?

@cderv
Copy link
Collaborator

cderv commented Jan 29, 2019

Is it possible to get the warnings inside the folds too?

I am looking into that. I think the folding should not be included in knitr but in rmarkdown or even bookdown for that.

@yihui
Copy link
Owner

yihui commented Jan 29, 2019

I think @cderv is correct: to fold all messages, this has to be implemented in the higher-level packages such as bookdown.

@cderv
Copy link
Collaborator

cderv commented Feb 2, 2019

Ok. So this is what we can get if we put the folding feature in bookdown:
https://travis-ci.com/cderv/adv-r/jobs/174867971

I think it is a pretty good result.

Here, book is rendered with quiet = TRUE and and knitr option for message, warning and error are set to FALSE. With this, we get rid of the progress information but warnings, messages and errors should appear inside travis folds.

The only thing indicating if a fold is empty or not, in the line numbers on the left.

@hadley is that closer to what you had in mind ?

@hadley
Copy link
Contributor Author

hadley commented Feb 4, 2019

@cderv given the massive reduction in output, folding no longer seems necessary. However if I use quiet = TRUE on the tidyverse style guide (https://travis-ci.org/tidyverse/style), I don't see quite enough output. So perhaps what I actually want is for the default bookdown output to display the file name being processed, and any errors/warnings/messages that are deliberately ignored.

@cderv
Copy link
Collaborator

cderv commented Feb 5, 2019

There is a few things I did in my previous example in addition of quiet = TRUE

  • Folding with printing about the file works only with new_session: yes in bookdown.yml or new_session = TRUE in render_book() so that a knit and merge approach comes into play.
    The bookdown default is a Merge and Knit approach and only a main Rmd file is built so no other files in being processed.
  • In order for errors/warnings/messages to print, knitr options must be set to FALSE. I have done it in a before_chapter_script bookdown.yml option with a script containing
    knitr::opts_chunk$set(message = FALSE, warning = FALSE, error = FALSE)
    This means any of these outputs will be printing in console (so travis log) instead of inside the knitted document.

Currently, style guide is in "Merge and Knit" approach (the default), so one file only in rendered. With quiet = FALSE, even with the modification I made in bookdown, it leads to nothing (no fold).
Also, there is no error / message / warning in the style guide currently. So nothing prints.

For the example, I added a dummy chunk with a warning in files.rmd. We get:

If the fold are not required, we could forget about the travis folding markers and just cat about the filename being process. Then activating this would mean:

  • Use a knit and merge approach to have file rendered on by one (new_session: yes)
  • Add the option for knitr chunks

If we want another behavior about how things are logged, this may need more changes in knitr, rmarkdown or bookdown.

There is an option progress in knitr I don't manage to use currently. maybe something could be done to reduce printing with progress = FALSE in knitr and quiet = FALSE in rmarkdown. I'll also look into this.

@yihui yihui added this to the v1.22 milestone Mar 7, 2019
@yihui
Copy link
Owner

yihui commented Mar 7, 2019

Closing this issue for now (happy to revisit if still needed).

@yihui yihui closed this as completed Mar 7, 2019
@github-actions
Copy link

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue by following the issue guide (https://yihui.org/issue/), and link to this old issue if necessary.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants