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

Use expect_no_message() over expect_silent() #198

Merged
merged 1 commit into from
Dec 15, 2022

Conversation

DavisVaughan
Copy link
Contributor

This PR makes your package compatible with the next version of dplyr:

The join functions in dplyr (like left_join()) now return a warning by default when a row in x matches multiple rows in y. While this is typical SQL behavior, it is often unexpected during data analysis (many people don't even know it is possible), so we've decided to make this a warning. In dplyr 1.1.0, you silence this warning with multiple = "all". In the meantime, we need to work around a broken test of yours that was expecting no output. I've done that by swapping expect_silent() for expect_no_message(), which I'm guessing is what you were trying to avoid.

We plan to submit dplyr 1.1.0 on January 27th.

This should be compatible with both dev and CRAN dplyr. It would help us out if you could go ahead and send a patch version of your package to CRAN ahead of time! Thanks!

@codecov
Copy link

codecov bot commented Dec 15, 2022

Codecov Report

Merging #198 (49f44ee) into main (84ee85b) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head 49f44ee differs from pull request most recent head 9697431. Consider uploading reports for the commit 9697431 to get more accurate results

@@            Coverage Diff             @@
##             main     #198      +/-   ##
==========================================
- Coverage   94.39%   94.37%   -0.02%     
==========================================
  Files          47       47              
  Lines        5580     5580              
==========================================
- Hits         5267     5266       -1     
- Misses        313      314       +1     
Impacted Files Coverage Δ
src/match-points.cpp 97.67% <0.00%> (-1.17%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@mpadge
Copy link
Member

mpadge commented Dec 15, 2022

Thank you very much for the attention and care @DavisVaughan. I'll merge straight away. This package was only recently updated on CRAN, so I dare not try another one so soon. But I'll make sure that i do it before 27th Jan. Thanks!

@mpadge mpadge merged commit 0b72f71 into UrbanAnalyst:main Dec 15, 2022
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