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

Add custom prefixes #7

Conversation

TimurSultanov
Copy link

Added an ability to add custom prefixes to the validator.

@coveralls
Copy link

coveralls commented Sep 17, 2019

Pull Request Test Coverage Report for Build 46

  • 14 of 14 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 42: 0.0%
Covered Lines: 248
Relevant Lines: 248

💛 - Coveralls

@TimurSultanov TimurSultanov reopened this Sep 17, 2019
@radarlog
Copy link
Contributor

@TimurSultanov thank you for the contribution.

The GTIN specification has defined all possible prefixes in a very precise way.

What's the purpose of you PR?

@TimurSultanov
Copy link
Author

TimurSultanov commented Sep 23, 2019

@radarlog We should use some custom prefixes in our company, for example "250"
One vendor creates GtINs with this prefix, and we should handle this situation.
Perhaps in the future we will have more custom prefixes.

@radarlog
Copy link
Contributor

According to the current implementation prefixes from the range 200-299 are used to issue Restricted Circulation Numbers within a company. According to the actual specification from above it still so (please check Figure 1.4.3-1)

@TimurSultanov
Copy link
Author

I see, but we need these prefixes for internal using.

@radarlog
Copy link
Contributor

Current implementation follows the GTIN specification as strict as possible.

Since you propose something very internal and specific to your business, I cannot accept your changes into the upstream.

I would rather suggest you to fork the library and adjust it according to your needs (as you already have actually done).

Hope for your understanding!

@TimurSultanov
Copy link
Author

Thank you

@radarlog radarlog closed this Sep 23, 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.

None yet

3 participants