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

export classes and add docs

Merged
merged 2 commits into from Jul 29, 2019
Merged

export classes and add docs #267

merged 2 commits into from Jul 29, 2019

Conversation

nuest
Copy link
Contributor

@nuest nuest commented Jul 29, 2019

see #248

setOldClass seems to require some manual definitions in roxygen, I followed https://stackoverflow.com/questions/7758748/documenting-setas-and-setoldclass-with-roxygen#7766762

jimhester added a commit that referenced this issue Jul 29, 2019
I did this in a simpler way than proposed in #267, I verfied this does
fix the WARNING when installing sos4R

Closes #367
@jimhester jimhester closed this in 4fa5bdf Jul 29, 2019
@jimhester
Copy link
Member

jimhester commented Jul 29, 2019

Thanks! I ended up doing this in a somewhat simpler fashion. I will release an updated version of xml2 next week with this fix included, and you can submit sos4R at that time.

@jimhester
Copy link
Member

jimhester commented Jul 29, 2019

Oh, now I see you get a note if the classes are undocumented, so I will actually just use this PR afterall :)

@jimhester jimhester reopened this Jul 29, 2019
@jimhester jimhester merged commit 784c8e5 into r-lib:master Jul 29, 2019
0 of 2 checks passed
@nuest
Copy link
Contributor Author

nuest commented Jul 29, 2019

😃

I did the same thing: "Oh, that was quick to fix" and then R CMD check comes along and gives you a slap on the wrist. 🤷‍♂️

@nuest
Copy link
Contributor Author

nuest commented Aug 1, 2019

@jimhester Do you plan a bugfix release for this? Otherwise I'll ask CRAN to admit sos4R despite the warning..

@jimhester
Copy link
Member

jimhester commented Aug 2, 2019

Yes, I will send a release early next week.

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

2 participants