Skip to content

Shinytest2#100

Merged
jepusto merged 21 commits intomainfrom
shinytest2
Mar 18, 2026
Merged

Shinytest2#100
jepusto merged 21 commits intomainfrom
shinytest2

Conversation

@Amycai224
Copy link
Copy Markdown
Collaborator

change shinytest to shinytest2

@jepusto
Copy link
Copy Markdown
Owner

jepusto commented Mar 6, 2026

Thanks. I still see two failing tests in test-SCD-effect-sizes.R. Could you take a look at those please?

Amy Cai and others added 4 commits March 9, 2026 20:07
Copy link
Copy Markdown
Owner

@jepusto jepusto 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 taking care of the remaining test failures. I've commented on a few points in the code where things can be cleaned up and simplified. Please attend to these and then clean up the test file by removing any extra comments that are not descriptive (e.g., # CHANGED and skip() calls that are commented out).

@jepusto jepusto self-requested a review March 14, 2026 16:38
Copy link
Copy Markdown
Owner

@jepusto jepusto left a comment

Choose a reason for hiding this comment

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

Please add your preferred name and email to DESCRIPTION.

@jepusto
Copy link
Copy Markdown
Owner

jepusto commented Mar 14, 2026

Please also remove the explicit call to xml2 in the shiny app tests, and remove xml2 from the SUGGESTS listing in DESCRIPTION.

@jepusto
Copy link
Copy Markdown
Owner

jepusto commented Mar 14, 2026

Please also remove the explicit call to xml2 in the shiny app tests, and remove xml2 from the SUGGESTS listing in DESCRIPTION.

The read_html() function from xml2 is available through rvest so there's no need to load both packages.

@jepusto
Copy link
Copy Markdown
Owner

jepusto commented Mar 17, 2026

Thanks. Please leave a comment to let me know when you're ready for me to review the PR again.

@Amycai224
Copy link
Copy Markdown
Collaborator Author

Thanks! I’ll do a final check to make sure everything is clean and will let you know when it’s ready for review

@Amycai224
Copy link
Copy Markdown
Collaborator Author

I’ve made the updates and everything should be ready now. Please feel free to review again. Thanks!

@jepusto jepusto merged commit 30a5d8c into main Mar 18, 2026
6 checks passed
@jepusto
Copy link
Copy Markdown
Owner

jepusto commented Mar 18, 2026

Closes #98.

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.

2 participants