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

Adjust the vin generation and validation algorithm - DO NOT MERGE #2678

Closed
wants to merge 1 commit into from
Closed

Adjust the vin generation and validation algorithm - DO NOT MERGE #2678

wants to merge 1 commit into from

Conversation

alextaujenis
Copy link
Contributor

Summary

The vehicle vin generator has been "refactored" and no longer produces valid vins, but all current tests are passing.

The changes proposed in #2640 will fix this issue and pin the correct behavior in the event the vehicle vin code is ever slightly changed or refactored.

Without the tests proposed in #2640 the vins generated by this library would have to be put through a separate third-party vin validator program to ensure any changes to this codebase do not corrupt the algorithm.

WARNING - DO NOT MERGE THIS CODE

This is a POC to show that the current tests for vehicle vins will not catch anyone refactoring the code. But #2640 fixes this issue and is ready to merge.

@alextaujenis alextaujenis changed the title Adjust the vin generation and validation algorithm Adjust the vin generation and validation algorithm - DO NOT MERGE Jan 3, 2023
@stefannibrasil
Copy link
Contributor

@alextaujenis with all due respect and gratitude for your interest in helping faker, the 3 open PRs are disruptive for the review process. Do you mind using your local branch for what you want to suggest and opening a PR when you're ready? That helps maintainers maintain the list of PRs to review straightforward. Thank you.

@alextaujenis
Copy link
Contributor Author

@stefannibrasil with all due respect I already opened a PR and it's ready - #2640. This is just supplemental testing for the VIN generator and validator in this library. I've explained what it is, why it's necessary, then showed you two examples exercising your test suite with passing and failing tests. I did this in two separate PRs so it would be easy for you to see, and honestly being a little disruptive got your attention with this issue you've let go stale and pushed the vulnerable code out in the 3.1.0 release.

I can't explain the situation in #2639 any more clearly, hoping to resolve the issue, while you pick apart how my code looks or how I'm not following your processes. I noticed you started to document the contribution guidelines for this project but most of it is placeholders, so I have no idea what your weird expectations are. I'm not paid to contribute to this library and had my local systems patched with my contribution before I even gave you the improved code. I'm not sure I can explain the issue or solution any other way that requires less mental energy to grasp, so I'm done here.

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