Skip to content
This repository has been archived by the owner on Jun 4, 2021. It is now read-only.

Add handling for leap day birthdays #220

Merged
merged 2 commits into from
May 14, 2018
Merged

Add handling for leap day birthdays #220

merged 2 commits into from
May 14, 2018

Conversation

hillaryj
Copy link
Contributor

We've been getting a ValueError for day out of range errors when a leap day birthday is entered in a non-leap year. This handles that error.

Testing

  1. Enter a leap day birthday such as 2/29/1952
  2. Complete the SSA tool estimation process
  3. You should get answers instead of a 500 error due to a ValueError

@hillaryj hillaryj self-assigned this May 14, 2018
@hillaryj hillaryj requested a review from higs4281 May 14, 2018 13:29
@coveralls
Copy link

coveralls commented May 14, 2018

Coverage Status

Coverage decreased (-0.1%) to 97.624% when pulling 60997dc on fix-leap-bday-error into 4330986 on master.

@hillaryj hillaryj requested a review from Scotchester May 14, 2018 13:35
Copy link
Contributor

@Scotchester Scotchester left a comment

Choose a reason for hiding this comment

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

This works for me, but please add a CHANGELOG entry before merging.

@Scotchester
Copy link
Contributor

Oh, please update the version number in package.json, too.

Hillary Jeffrey and others added 2 commits May 14, 2018 13:13
We've been getting a ValueError for day out of range errors when a leap day birthday is entered in a non-leap year. This handles that error.
@Scotchester
Copy link
Contributor

Went down a Snyk rabbit hole, and also discovered that this branch was a ways behind master, so I updated a bunch of stuff and force-pushed it.

@hillaryj hillaryj merged commit 404038a into master May 14, 2018
@hillaryj hillaryj deleted the fix-leap-bday-error branch May 14, 2018 18:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants