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

cran release 010 #73

Merged
merged 14 commits into from
Aug 15, 2022
Merged

cran release 010 #73

merged 14 commits into from
Aug 15, 2022

Conversation

zkamvar
Copy link
Member

@zkamvar zkamvar commented Aug 8, 2022

This PR will be merged once tinkr is released on CRAN

@zkamvar zkamvar mentioned this pull request Aug 9, 2022
@zkamvar
Copy link
Member Author

zkamvar commented Aug 12, 2022

Got some feedback:

Please do not start the description with "This package", package name,
title or similar.

"Using foo:::f instead of foo::f allows access to unexported objects.
This is generally not recommended, as the semantics of unexported
objects may be changed by the package author in routine maintenance."
Please omit one colon.

You have examples for unexported functions. -> man/yarn.Rd
Please either omit these examples or export these functions.

Please choose a more meaningful vignette title and not "Untitled".

Please ensure that your functions do not write by default or in your
examples/vignettes/tests in the user's home filespace (including the
package directory and getwd()). This is not allowed by CRAN policies.
Please omit any default path in writing functions.
In your examples/vignettes/tests you can write to tempdir().

Please fix and resubmit.

A lot of this is not relevant to {tinkr} (e.g description and vignette) and I have no clue why they got inserted, but some of this is (using non-exported functions in help documentation to document those functions), but it oddly points to the wrong documentation file.

I think the tests point is legit, though, so I'll have a looksee.

@zkamvar
Copy link
Member Author

zkamvar commented Aug 12, 2022

Here is an example of a test file that does not follow the normal tempfile conventions:

test_that("to_md fails if the stylesheet is not correct", {
tmp <- withr::local_file("dummy.xml")

@zkamvar zkamvar marked this pull request as ready for review August 13, 2022 00:26
@zkamvar
Copy link
Member Author

zkamvar commented Aug 15, 2022

Response from CRAN:

Thanks,

 > Please do not start the
    description with "This package", package name, title
    or similar.

Your description starts with:
     "Casts '(R)Markdown' files to 'XML' and back to..."
Which is essentially your title.

 >Please choose a more meaningful vignette title and
   not "Untitled".

While you don't have an untitled vignette, there are a lot of .Rmd files
in your /inst and /test directories which seem to be unnecessary. Please
check if you really need them all or if some of them can be omitted.

Please fix and resubmit.

 - ignore several files in .Rbuildignore
 - move stylesheets to specific `stylesheets/` folder
 - update code to reflect change
@zkamvar
Copy link
Member Author

zkamvar commented Aug 15, 2022

Note: I'm going to merge this because, while it is not complete, I got the following comment from Uwe:

Thanks, we see:

   Found the following (possibly) invalid URLs:
     URL:
https://github.com/ropensci/tinkr/blob/main/inst/stylesheets/xml2md_gfm.xsl
       From: inst/doc/tinkr.html
             README.md
       Status: 404
       Message: Not Found

Please fix and resubmit.

And of course this URL will be valid once it is merged into main. So I'm going
to decrement the version number, merge, change back to this branch, and increment
the version number again and submit.

@zkamvar zkamvar merged commit 9aeeaf9 into main Aug 15, 2022
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.

None yet

1 participant