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

Refactor of CONTRIBUTING.md #96

Merged
merged 4 commits into from
Dec 1, 2020
Merged

Refactor of CONTRIBUTING.md #96

merged 4 commits into from
Dec 1, 2020

Conversation

bram-pkg
Copy link
Member

@bram-pkg bram-pkg commented Dec 1, 2020

What is the reason for this PR?

  • A new feature
  • Fixes issue #

Author's checklist

Summary of changes

The contribution guide was quite outdates, I have updated links and rewritten parts that refer to documenting in the README.md.

Review checklist

  • All checks have passed
  • Changes are approved by maintainer

@bram-pkg bram-pkg added the documentation Improvements or additions to documentation label Dec 1, 2020
@bram-pkg bram-pkg requested a review from pimjansen December 1, 2020 19:40
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
@bram-pkg bram-pkg marked this pull request as ready for review December 1, 2020 21:48
@GrahamCampbell
Copy link
Member

Something seems up with the national insurance stuff. Either we have flakey generation, or a flakey test?

@bram-pkg
Copy link
Member Author

bram-pkg commented Dec 1, 2020

I have no clue, as you can see nothing regarding that has changed and it's passing on main.

Running again has seemed to fix it, so definitely inconsistent tests.

@bram-pkg
Copy link
Member Author

bram-pkg commented Dec 1, 2020

I found the cause, it's not the bank account numbers, it's the new test for en_GB::nino:

1) Faker\Test\Provider\en_GB\PersonTest::testNationalInsuranceNumber
Failed asserting that true is false.

@pimjansen
Copy link

I raised a bug for this to review since it is not consistent it seems

@bram-pkg
Copy link
Member Author

bram-pkg commented Dec 1, 2020

Issue with the test is fixed in main. @GrahamCampbell

@GrahamCampbell GrahamCampbell merged commit e942d5a into FakerPHP:main Dec 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants