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: Vehicle vin is always 17 characters long #320

Merged
merged 5 commits into from
Mar 28, 2022

Conversation

demipel8
Copy link
Contributor

@demipel8 demipel8 commented Jan 27, 2022

Solves #319

@demipel8 demipel8 requested a review from a team as a code owner January 27, 2022 11:28
test/vehicle.spec.ts Outdated Show resolved Hide resolved
test/vehicle.spec.ts Outdated Show resolved Hide resolved
@import-brain import-brain added the c: bug Something isn't working label Jan 27, 2022
import-brain
import-brain previously approved these changes Jan 27, 2022
@import-brain
Copy link
Member

Please merge those tests. Other than that, this lgtm!

@import-brain import-brain added the p: 2-high Fix main branch label Jan 27, 2022
@Shinigami92
Copy link
Member

This is a runtime change bug => 6.1+

ST-DDT
ST-DDT previously approved these changes Jan 28, 2022
import-brain
import-brain previously approved these changes Jan 28, 2022
@demipel8 demipel8 dismissed stale reviews from import-brain and ST-DDT via a0fb449 January 31, 2022 09:45
ST-DDT
ST-DDT previously approved these changes Jan 31, 2022
import-brain
import-brain previously approved these changes Jan 31, 2022
test/vehicle.spec.ts Outdated Show resolved Hide resolved
@Shinigami92 Shinigami92 added the needs rebase There is a merge conflict label Mar 10, 2022
@Shinigami92
Copy link
Member

@demipel8 This PR needs a rebase

@demipel8 demipel8 dismissed stale reviews from import-brain and ST-DDT via df25d67 March 11, 2022 08:35
@demipel8 demipel8 force-pushed the fix-vehicle-vin-length branch 5 times, most recently from a0fb449 to 7a6e23c Compare March 11, 2022 09:04
@Shinigami92 Shinigami92 removed the needs rebase There is a merge conflict label Mar 11, 2022
@Shinigami92
Copy link
Member

@demipel8 Tests are running red

@Shinigami92 Shinigami92 added p: 1-normal Nothing urgent and removed p: 2-high Fix main branch labels Mar 15, 2022
@Shinigami92 Shinigami92 linked an issue Mar 23, 2022 that may be closed by this pull request
@ST-DDT ST-DDT force-pushed the fix-vehicle-vin-length branch from 62733d5 to 951aa91 Compare March 28, 2022 15:09
@ST-DDT ST-DDT added the s: accepted Accepted feature / Confirmed bug label Mar 28, 2022
ST-DDT
ST-DDT previously approved these changes Mar 28, 2022
@ST-DDT ST-DDT requested review from a team March 28, 2022 15:18
@codecov
Copy link

codecov bot commented Mar 28, 2022

Codecov Report

Merging #320 (02f4b88) into main (94c96ba) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 02f4b88 differs from pull request most recent head 13d0aeb. Consider uploading reports for the commit 13d0aeb to get more accurate results

@@           Coverage Diff           @@
##             main     #320   +/-   ##
=======================================
  Coverage   99.34%   99.34%           
=======================================
  Files        1923     1923           
  Lines      176900   176890   -10     
  Branches      901      897    -4     
=======================================
- Hits       175739   175735    -4     
+ Misses       1105     1099    -6     
  Partials       56       56           
Impacted Files Coverage Δ
src/vehicle.ts 100.00% <100.00%> (ø)
src/index.ts 94.87% <0.00%> (-2.57%) ⬇️
src/random.ts 99.44% <0.00%> (-0.01%) ⬇️
src/faker.ts 100.00% <0.00%> (ø)
src/lorem.ts 100.00% <0.00%> (ø)
src/system.ts 96.48% <0.00%> (+0.03%) ⬆️
src/vendor/user-agent.ts 98.03% <0.00%> (+0.28%) ⬆️
src/name.ts 100.00% <0.00%> (+0.30%) ⬆️
src/finance.ts 100.00% <0.00%> (+0.69%) ⬆️
src/git.ts 100.00% <0.00%> (+1.69%) ⬆️

@ST-DDT ST-DDT requested a review from a team March 28, 2022 16:35
@ST-DDT ST-DDT requested review from a team March 28, 2022 17:08
@Shinigami92 Shinigami92 enabled auto-merge (squash) March 28, 2022 17:20
@Shinigami92 Shinigami92 merged commit d2fc1e6 into faker-js:main Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: bug Something isn't working p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

faker.vehicle.vin() returning an invalid Vin number
8 participants