Skip to content

Roxygen cache not invalidated when function signature changes #213

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

Closed
lorenzwalthert opened this issue Oct 26, 2020 · 4 comments · Fixed by #214
Closed

Roxygen cache not invalidated when function signature changes #213

lorenzwalthert opened this issue Oct 26, 2020 · 4 comments · Fixed by #214

Comments

@lorenzwalthert
Copy link
Owner

Since we only check roxygen2 comments, we don't invalidate the cache currently when the function header changes, e.g. from

#' comment
f <- function(x) {
  ...
}

to

#' comment
f <- function(x, y) {
  ...
}
@lorenzwalthert
Copy link
Owner Author

This might be also a problem with R6. @pat-s have you experienced problems here? Looks like with this PR, any change in a line (after comment removal) with function( in it should trigger an invalidation so we are safe.

Also see https://roxygen2.r-lib.org/articles/rd.html#r6

@pat-s
Copy link
Contributor

pat-s commented Nov 1, 2020

This means a change in a signature would not cause a re-styling? I don't think I've experience this in a way that I would make me suspicious something is going wrong.

AFAIU one would not recognise a direct failure but just styling silently being skipped because the cache is used, correct?

@lorenzwalthert
Copy link
Owner Author

lorenzwalthert commented Nov 1, 2020

No, this refers to the roxygenize hook, not to the styler hook. If you change the function declaration without changing any roxygen comment, the cache of the roxygenize hook in R precommit (not styler) is not invalidated with the master branch currently because we only check if the diff contains the regex ^#'.

@lorenzwalthert
Copy link
Owner Author

Another one is r-lib/styler@1e9f254. Not sure what's the problem.

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

Successfully merging a pull request may close this issue.

2 participants