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

Fix round number for Wednesday matches #96

Merged
merged 2 commits into from
Dec 9, 2019
Merged

Fix round number for Wednesday matches #96

merged 2 commits into from
Dec 9, 2019

Conversation

cfranklin11
Copy link
Contributor

Resolves #95

As with other recent bugs for footywire round number calculations,
I'm a bit confused as to what changed regarding how weeks and
weekdays are counted to cause this, but something moved Wednesdays
from the beginning to the end of the round week by default.

Changing the between filter to use Sunday and Tuesday as the limits
makes the default round week Wednesday to Tuesday as intended.

@jimmyday12
Copy link
Owner

Looks like there might be a new issue with AFLW data that is causing all tests to fail at the moment :( I might have to refactor that part of the code before merging this in

@jimmyday12
Copy link
Owner

Ok made a commit (8fe62d2) that fixes the womens-stats error on Travis. If you merge master into your PR and resubmit it should be fine.

Also, just to note, to get tests to run on your local machine you have to do the following code since we are using skip_on_cran() in our tests.

Sys.setenv(NOT_CRAN = "true")
devtools::check()

Related to this issue that should be fixed soon in devtools

As with other recent bugs for footywire round number calculations,
I'm a bit confused as to what changed regarding how weeks and
weekdays are counted to cause this, but something moved Wednesdays
from the beginning to the end of the round week by default.

Changing the between filter to use Sunday and Tuesday as the limits
makes the default round week Wednesday to Tuesday as intended.
@cfranklin11
Copy link
Contributor Author

Ok, rebased and pushed, and it looks like it's good to merge

@jimmyday12 jimmyday12 merged commit 47bf232 into jimmyday12:master Dec 9, 2019
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.

2019 rounds 5/6 have incorrect matches
2 participants