AndreaCirilloAC / updateR

update your R version in a breeze ( on OSX) √
Other
143 stars 23 forks source link

push to CRAN #7

Open AndreaCirilloAC opened 7 years ago

AndreaCirilloAC commented 7 years ago

@RobertMyles as you previously suggested I think this useful package is now ready to be submitted to CRAN. What do you think about it?

RobertMyles commented 7 years ago

I think it's good to go, Andrea! Maybe we could run some checks with the devtools package before we submit it, but other than that I think it's good to go :-)

AndreaCirilloAC commented 7 years ago

I run the devtools check on the package. everything went smoothly except for the following error:

* checking package dependencies ... ERROR
Namespace dependency not required: ‘magrittr’

See section ‘The DESCRIPTION file’ in the ‘Writing R Extensions’
manual.
* DONE
Status: 1 ERROR
checking package dependencies ... ERROR
Namespace dependency not required: ‘magrittr’

going to look into this.

RobertMyles commented 7 years ago

Ah, it must be the line "@importFrom magrittr '%>%' "; it appears to be unnecessary; my mistake!

AndreaCirilloAC commented 7 years ago

I was looking at that as well. Nevertheless I was wondering: if we remove the dependency from magrittr from when will the package import the pipe operator?

Il giorno 28 mar 2017, alle ore 14:38, Robert Myles McDonnell notifications@github.com ha scritto:

Ah, it must be the line "@importFrom magrittr '%>%' "; it appears to be unnecessary; my mistake!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

RobertMyles commented 7 years ago

Yeah, that's a strange one. We use the pipe, so I think it's better to explicitly import it, and I'm not sure why the check would throw an error like that. Looking at the code, I think we should probably also change the example to a "Not run" example. Perhaps we could try changing back to importing dplyr instead of magrittr and see if it throws this error?

RobertMyles commented 7 years ago

Hi Andrea, I just pushed a commit that should fix some of these problems. I didn't change the example, maybe you might like to do that, but it is pretty much ready to go to CRAN.

AndreaCirilloAC commented 7 years ago

thank you @RobertMyles , just submitted. going to wait for their review for (hopefully) closing the issue. schermata 2017-04-04 alle 05 11 17

AndreaCirilloAC commented 7 years ago

nearly done, but still not there, got the following two errors from pre-test:

full log at:https://win-builder.r-project.org/incoming_pretest/170404_051110_updateR_01/00check.log

this is due to platform limitation I guess. going to look how to overcome this, which I think is possible since packages like our inspiration InstallR from Tal Galili are for Windows only.

AndreaCirilloAC commented 7 years ago

added os_type on description with https://github.com/AndreaCirilloAC/updateR/commit/fad7ff31ffdb62dc20c844ad2b216526a106ae20