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

Data Documentation for load_espn_qbr #74

Merged
merged 7 commits into from
Dec 31, 2021
Merged

Data Documentation for load_espn_qbr #74

merged 7 commits into from
Dec 31, 2021

Conversation

pranavrajaram
Copy link
Contributor

Hey guys,

I thought I'd try and give back to the nflreadr because of how helpful it is, so I started to work on one of the "good first issues" that were listed. To start, I created a data dictionary for the "load_espn_qbr" package which is what this pull request is for. Everything seems to work fine on my end in R (if I load in my forked github using devtools), but I'm not sure on how to test if my updates are also shown in the pkgdown website.

Please let me know if I did anything wrong, and any feedback would be much appreciated. Thanks for all you do for the community!

@tanho63
Copy link
Member

tanho63 commented Dec 31, 2021

Awesome! You can check locally to see if the pkgdown will build by running devtools::build_site() (or pkgdown's equivalent). I'll have a quick peek here around lunch

R/load_nflverse.R Outdated Show resolved Hide resolved
@tanho63
Copy link
Member

tanho63 commented Dec 31, 2021

Can you also run devtools::document() twice?

Copy link
Member

@tanho63 tanho63 left a comment

Choose a reason for hiding this comment

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

See typo note + please redocument with devtools::document() (usually twice), then commit/push those here and the checks should pass

@tanho63 tanho63 enabled auto-merge (squash) December 31, 2021 16:19
@tanho63 tanho63 mentioned this pull request Dec 31, 2021
11 tasks
@pranavrajaram
Copy link
Contributor Author

thanks for getting back! might be a dumb question, but where should I run devtools::document()?

@tanho63
Copy link
Member

tanho63 commented Dec 31, 2021

Just in your console, it'll operate over the whole package

auto-merge was automatically disabled December 31, 2021 16:32

Head branch was pushed to by a user without write access

Copy link
Member

@tanho63 tanho63 left a comment

Choose a reason for hiding this comment

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

LGTM

@tanho63 tanho63 enabled auto-merge (squash) December 31, 2021 16:39
@tanho63 tanho63 merged commit 4379d17 into nflverse:main Dec 31, 2021
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