Skip to content

Conversation

@olivroy
Copy link
Contributor

@olivroy olivroy commented Jan 29, 2025

Also removes unused suggests

add a search bar to website also

The added imports don't change anything to the package, since they are already imported by ggplot2.

I wanted to see if I could remove the {plyr} dependency, but I couldn't. The PR is already big as it is.

@olivroy
Copy link
Contributor Author

olivroy commented Jun 3, 2025

Hi @eliocamp ! Let me know what you think or if you need more explanation / PR split

@eliocamp
Copy link
Owner

Hi! Yes, it is a pretty large PR that does a lot of different stuff. It would be great if you could split it into more atomic PRs.

olivroy added 2 commits June 25, 2025 11:56
(I know the diff is bad from GitHub, but I verified that they matched when I first opened the PR.
@olivroy olivroy changed the title Update to testthat 3, avoid partial match, update ggplot2 code Update to testthat 3 Jun 25, 2025
@olivroy
Copy link
Contributor Author

olivroy commented Jun 25, 2025

I modified this PR to be only testthat 3. Opened another one for partial matching. I will see if I re-implement the other changes. The move to testthat 3 may help with future proofing the package

@eliocamp
Copy link
Owner

eliocamp commented Jul 1, 2025

I'm not versed in snapshot testing. The expect_snapshot() expectation test for the printout. But in many cases what the test was testing was the object. Shouldn't it be expect_snapshot_value() instead?

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