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

Disable roxygen comments processing for unavailable methods #342

Merged
merged 2 commits into from
Nov 27, 2018

Conversation

jakobzeitler
Copy link

I had been, unfortunately, looking at outdated sample code for grf which included a use of "create_dot_body" which has been moved into a sub-method for plot.grf_tree since. As "create_dot_body" was listed in the 00index.html, I was under the assumption it should be available as a method even though it was not. Maybe this commit will help prevent confusion of that kind while still maintaining the commentary for development purposed. Would this comply with the GRF coding/documentation practices?

I had been looking at outdated sample code which included a use of "create_dot_body" which has been used as a sub-method for plot.grf_tree since. As "create_dot_body" was listed in the 00index.html, I was under the assumption it should be available as a method even though it was not. Maybe this will help prevent confusion of that kind while still maintaining the commentary. Would this comply with the GRF coding/documentation practices?
@jtibshirani
Copy link
Member

Thanks @jakobzeitler for catching this issue. I think that instead of disabling roxygen processing altogether, we should add the tag @keyword internal to prevent the function from being included in the package index, as suggested in https://cran.r-project.org/web/packages/roxygen2/vignettes/rd.html.

I was also wondering what outdated sample code you encountered (in case there's an example we should make sure to update somewhere)?

@jakobzeitler
Copy link
Author

Thanks for the response, Julie. I had been looking for the @Keyword internal to solve this, thanks for pointing that out!

I was also looking for the example code I used, that included the "create_dot_body", but I have not found it, yet. (A repository wide search did not return anything, so it must have originated somewhere outside). I will let you know when it comes back to my mind.

@jakobzeitler
Copy link
Author

I have added the keyword to the plot.R. Do you have any other methods in mind that would require that keyword? I can add it there as well, if required.

Copy link
Member

@jtibshirani jtibshirani left a comment

Choose a reason for hiding this comment

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

This looks good to me. I don't see any other methods that should be marked as internal.

@jtibshirani jtibshirani merged commit d15e99f into grf-labs:master Nov 27, 2018
jtibshirani added a commit that referenced this pull request Aug 14, 2019
In #342 we tried to exclude private helper methods from being included in the
package documentation. However we accidentally used '@Keyword' instead of the
valid directive '@Keywords'.
jtibshirani added a commit that referenced this pull request Aug 14, 2019
In #342 we tried to exclude private helper methods from being included in the
package documentation. However we accidentally used `@keyword` instead of the
valid directive `@keywords`.
jtibshirani added a commit that referenced this pull request Aug 15, 2019
In #342 we tried to exclude private helper methods from being included in the
package documentation. However we accidentally used `@keyword` instead of the
valid directive `@keywords`.
jtibshirani added a commit that referenced this pull request Aug 19, 2019
In #342 we tried to exclude private helper methods from being included in the
package documentation. However we accidentally used `@keyword` instead of the
valid directive `@keywords`.
davidahirshberg pushed a commit to davidahirshberg/grf that referenced this pull request Dec 6, 2019
In grf-labs#342 we tried to exclude private helper methods from being included in the
package documentation. However we accidentally used `@keyword` instead of the
valid directive `@keywords`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants