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

option for not writing roxygen2 pkg version to Rd files #334

Closed
wants to merge 1 commit into from
Closed

option for not writing roxygen2 pkg version to Rd files #334

wants to merge 1 commit into from

Conversation

jangorecki
Copy link

Setting option below will resolve following SO question: R render Rd using roxygen2 without roxygen2 version. Didn't create separate issue here on github for such feature, I may if that would helps.

options("roxygen2.pkg.version" = FALSE)

Feel free to modify approach at your will, reject, develop own way. I'm just interested to see that possibility.
Also if you incorporate that feature you may answer SO and grab bounty 👍

IMO it could be even default option, rewriting doc files when nothing changed in their content, but only roxygen2 version changed, is not a great approach.

@hadley
Copy link
Member

hadley commented Apr 25, 2015

I don't think options is the right place for this because you want it to be shared across all collaborators on a project, so it should be a setting in DESCRIPTION.

Alternatively, maybe the goal of this feature is best served another way. The motivation for adding this comment was to make sure that everyone developing the package is using the same version of roxygen, so it's very obvious when two people are using conflicting versions. Maybe instead of writing it in every file, roxygen could just write it in one file - there will still be one change when upgrade roxygen, but that is still too much you could .gitignore that file.

@yihui will be excited about this feature too.

@jangorecki
Copy link
Author

Any way will be a good way. This was just a fast and easy proposal. Your suggestion of own technical file seems nice, roxygen could store any roxygen related technical information which in future could also includes parser and options used.

@yihui
Copy link
Contributor

yihui commented Apr 25, 2015

Definitely :)

@leeper
Copy link

leeper commented Apr 28, 2015

Could I propose an alternative that might be simpler? Currently, roxygen checks whether an existing .Rd file matches the contents that would be overwritten and, if they match, nothing is changed. If the files don't match, then the contents overwrite the existing file (see https://github.com/klutometis/roxygen/blob/009ae8bdac7580db23e6ad00c98cd65ee4375fe6/R/utils.R#L114-L131).

If this comparison skipped the first line of the .Rd file (and the first line of the contents), then the comparison would ignore the roxygen version information. This would allow each Rd file to have been created by different versions of roxygen as long as those version produce identical contents. Seems like it would require at most a relatively modest change to the same_contents function and avoid the need to modify package behavior at a more fundamental level.

@hadley
Copy link
Member

hadley commented Apr 28, 2015

@leeper to me, that would be confusing - the goal of the version string is to ensure that all authors of the package are using the same version of roxygen2.

@leeper
Copy link

leeper commented Apr 28, 2015

@hadley Sure, but the version notes would only differ when changes between roxygen2 versions are substantively inconsequential. This could actually be clearer because documentation pages would only change when there were substantive differences between the output of different roxygen2 versions.

@hadley
Copy link
Member

hadley commented Apr 28, 2015

@leeper I think it's a bad idea because the recorded version then just becomes the first version of roxygen used with that file. Over time, the files will have a mix of recorded versions. I don't see why that is preferable to recording in one place what version of roxygen2 was used.

@leeper
Copy link

leeper commented Apr 28, 2015

@hadley That seems like a good thing to me (different versions in different
files) but you don't need to see it that way since it's your package not
mine. I think what I'm proposing is better than the current behavior that
this PR is aiming to solve. I'm not sure if it's better than recording the
version in one place (like in DESCRIPTION or elsewhere), but it would at
least be a backwards compatible change.

On Tue, Apr 28, 2015 at 2:32 PM, Hadley Wickham notifications@github.com
wrote:

@leeper https://github.com/leeper I think it's a bad idea because the
recorded version then just becomes the first version of roxygen used with
that file. Over time, the files will have a mix of recorded versions. I
don't see why that is preferable to recording in one place what version of
roxygen2 was used.


Reply to this email directly or view it on GitHub
#334 (comment).

@jangorecki
Copy link
Author

% Please edit documentation in R/*.R
This could also be improved, as it force to update man files on the R scripts rename.

@krlmlr
Copy link
Member

krlmlr commented Jun 4, 2015

@leeper @hadley Why not do both: Don't touch the first line if the file hasn't changed otherwise, and write new version information only in NAMESPACE? The version information from the .Rd files will gradually phase out, and can be purged by the user simply by removing and recreating all files in man.

@krlmlr
Copy link
Member

krlmlr commented Jun 4, 2015

@jangorecki But if you rename the script, the back reference in the comment becomes invalid. I think those back references are mostly a good thing, and renames of script files are rare. (They could even be used by an IDE hint hint to list and allow the user to open the source files that correspond to a given documentation page.)

@jangorecki
Copy link
Author

@krlmlr I know but I don't need back reference at all. I can lookup where to function is defined or use CTRL + left mouse to get to the source file. I understand it may be useful if somebody would starts to updating man/*.Rd instead of roxygen metadata doc. But as long as 1) I don't have much PR, 2) I'm gatekeeper for each PR, 3) people who do PR are usually aware the docs might have been written using roxygen so they can check it before writing Rd directly, I'm pretty fine on getting rid the back reference which adds a noise to source control.

@krlmlr
Copy link
Member

krlmlr commented Jun 4, 2015

@jangorecki Perhaps 1) 2) and 3) don't apply to every project that uses roxygen2. I think it's somewhat helpful, and there's not too much "source control noise" (but then I'm biased because this feature is my contribution).

Why don't you submit a pull request that makes the back references optional via the Roxygen field in DESCRIPTION? (It's still up to the maintainers to decide.)

@jangorecki
Copy link
Author

Surely not, that's why I've suggested an option. It is not a big deal for me as I'm documenting with modified roxygen. I would call it a noise as long as maintainer is not interested in tracking back references. Renaming *.R files is already caught by git. Updating back reference make sense, but if somebody wants to use track it.

@hadley
Copy link
Member

hadley commented Oct 4, 2015

I don't like this solution but the next version will definitely not add the version to every file by default

@hadley hadley closed this Oct 4, 2015
jangorecki referenced this pull request in h2oai/sparkling-water Oct 18, 2016
[MINOR][RSparkling] Update docs
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.

5 participants