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

Roxygen fix #19

Merged
merged 13 commits into from
Aug 26, 2022
Merged

Roxygen fix #19

merged 13 commits into from
Aug 26, 2022

Conversation

andy-techen
Copy link
Contributor

I've mainly made two changes to the roxygen SCTs:

  • Removed the deprecated and removed registry argument in parse_text() (used in extract_roxygen_from_code()).
  • Apparently, roxygen no longer allows us to directly index the parsed blocks. I'm guessing this might have something to do with our removal of registry. Therefore, I resorted to using the block functions in roxygen to extract tags from the state.

Most likely, there's a better way to do this (tag functions might be something worth digging into). That being said, I've tested out the updated SCTs in Teach and everything seems to be working fine! Please let me know if this seems like a valid solution to you!

Copy link
Contributor

@TimSangster TimSangster left a comment

Choose a reason for hiding this comment

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

Thanks for keeping us in the loop, sadly I don't know enough R to review fully, will wait for somebody else to review.
Do you have a way to test these changes? i.e. did you locally test it on some code or copy paste it into exercises to check it works correctly? Thank you!

@andy-techen
Copy link
Contributor Author

@TimSangster No worries whatsoever! And yes, I copied the functions into the SCTs and the exercises seem to be passing!

@andy-techen
Copy link
Contributor Author

@richierocks I've added some fixes to the package check functions, as well as the unused equal check functions. The tests in test-check-roxy.R seem to be passing locally right now! Please let me know if the functions are still failing on your end! Thank you!

@richierocks
Copy link
Contributor

I spotted some tests that were being skipped in the latest testthat. Fixing those meant I found a few cases where the state wasn't being returned from the check function. And there was some gnarly sapply() code I'd written years ago that I swapped out for cleaner purrr code. All looking good now, and test are passing.

@TimSangster Merge at your leisure.

@TimSangster TimSangster merged commit 6b9f25d into master Aug 26, 2022
@TimSangster TimSangster deleted the roxygen_fix branch August 26, 2022 07:46
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

3 participants