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

Add ebird-checklist #107

Closed

Conversation

RichardLitt
Copy link
Contributor

@RichardLitt RichardLitt commented Feb 1, 2024

Description

Apologies for the PR template mess-up: I was using my terminal and meant to PR to @Rafnuss's PR in #93. I added a test to his and added magrittr, which was messing up. See #93.

This is my first PR to a project using R, so let me know if I've messed anything up.

Related Issue

Example

@RichardLitt RichardLitt changed the title <!-- IF THIS INVOLVES AUTHENTICATION: DO NOT SHARE YOUR USERNAME/PASSWORD, OR API KEYS/TOKENS IN THIS ISSUE - MOST LIKELY THE MAINTAINER WILL HAVE THEIR OWN EQUIVALENT KEY --> Add ebird-checklist Feb 1, 2024
@RichardLitt
Copy link
Contributor Author

Note: I've opened a PR with this content on #93 now, so if that's merged we can just close this one.

@slager
Copy link
Collaborator

slager commented Feb 2, 2024

@RichardLitt Your fork is based on an old version of rebird. You'll first need to merge in the more recent changes from rebird/master into your fork and resolve any merge conflicts.

Once you do that, the new continuous integration tests should show up in this PR.

Based on the failed tests in #93 you'll also probably need to add Roxygen documentation for the sleep argument.

Rather than add magrittr as a new dependency to rebird, I'd recommend removing the magrittr stuff and using the new base R pipe |>

@RichardLitt RichardLitt force-pushed the add-ebirdchecklist-tests branch from 9792436 to cc2c33f Compare February 2, 2024 15:08
@slager
Copy link
Collaborator

slager commented Feb 2, 2024

@RichardLitt We're still working lately on getting the automated testing working with the key (#104) or without a key (#105) on GitHub Actions. You could add a skip_on_ci() inside your testthat block for now (see #101 for examples) so that it doesn't fail due to no key. That way the CI will at least do an R CMD check on your function.

@RichardLitt RichardLitt closed this Feb 2, 2024
@RichardLitt RichardLitt force-pushed the add-ebirdchecklist-tests branch from a2d1ab6 to 4dfa145 Compare February 2, 2024 15:41
@RichardLitt RichardLitt mentioned this pull request Feb 2, 2024
@RichardLitt
Copy link
Contributor Author

@slager Done everything, I think. See #108.

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