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

Remove logical condition recycling from fcase() #6363

Merged
merged 6 commits into from
Aug 21, 2024

Conversation

MichaelChirico
Copy link
Member

Closes #6352 as discussed.

Not a big fan of the need for suppressWarnings() in the R body; ...length() is the way to avoid this, but that's not available till R3.5.0.

Copy link

github-actions bot commented Aug 12, 2024

Comparison Plot

Generated via commit 4459862

Download link for the artifact containing the test results: ↓ atime-results.zip

Time taken to finish the standard R installation steps: 12 minutes and 11 seconds

Time taken to run atime::atime_pkg on the tests: 6 minutes and 11 seconds

Copy link
Member

@tdhock tdhock left a comment

Choose a reason for hiding this comment

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

looks good, nice NEWS item.
you may want to ask @DavisVaughan for review too?

@MichaelChirico
Copy link
Member Author

I'm not sure he's reviewed code here before so it might take some effort to parse through it. I think the high-level feedback he gave leading to this was clear enough & implemented here. But Davis, do feel free to chime in!

@MichaelChirico MichaelChirico merged commit 26c2c08 into master Aug 21, 2024
4 of 5 checks passed
@MichaelChirico MichaelChirico deleted the fcase-norecycle-whens branch August 21, 2024 04:01
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.

Consider reverting fcase recycled conditions
2 participants