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

Small bugfix to prevent available.versions crashing early #22

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

hughjonesd
Copy link
Contributor

when a version is not recognized.

  • Alternative approach would be to have current_version return a 0-row data frame rather than a 1-row dataframe with NAs.
  • The underlying issue is that all(NA) returns NA on a one-row dataset.

…ion is not recognized.

* Alternative approach would be to have current_version return a 0-row data frame rather than a 1-row dataframe with NAs.
@goldingn goldingn mentioned this pull request Mar 4, 2018
@goldingn
Copy link
Owner

goldingn commented Mar 5, 2018

Hi David,

Could you please add an issue showing where this causes you an error, so I replicate it?

I can see why that line of code would error if a single row dataframe is returned with NA in the available column, just not what would lead to the NA.

Nick

@hughjonesd
Copy link
Contributor Author

The one row dataframe is returned from current_version if version is not recognized. Maybe that's fine and you want to throw an error, but it would be nice to have a more comprehensible one; and your warning suggests you want to recover from a single bad version.

> versions::available.versions(c("bad", "testthat"))
Error in if (!all(df$available)) { : 
  missing value where TRUE/FALSE needed
In addition: Warning message:
In current.version(pkgs) :
  The current version and publication date of bad could not
                     be detected

@goldingn
Copy link
Owner

goldingn commented Mar 5, 2018

Gotcha, yep that needs a better error!

@hughjonesd
Copy link
Contributor Author

OK, this tries to throw an error earlier. The alternative would be to do it in current_version.

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