chore(typedefs): added typedefs #37
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added type definitions to the results for MRZ as well as consolidated all the types into a singular file and exported them from the index.ts file for ease of use with developers. also added comments and documentation where possible and some mild lint clean up
stropitek
requested changes
Mar 23, 2023
| } | ||
|
|
||
| /** | ||
| * It takes a string and returns a string |
There was a problem hiding this comment.
This description is not helpful (just describes what already is understandable from the type anotations)
|
|
||
| /** | ||
| * It takes a string and returns a string | ||
| * @param {string} char - The character to convert. |
There was a problem hiding this comment.
- The params don't need type annotations in JS doc since they are available from the TS types.
- The param description doesn't add helpful context (the function name
letterToNumberalready says it converts a char.
| /** | ||
| * It takes a string and returns a string | ||
| * @param {string} char - The character to convert. | ||
| * @returns A function that takes a string and returns a string. |
There was a problem hiding this comment.
- The return value does to need type annotation with JS doc since already available from TS
- The description is not helpful
| corrected: string; | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Could you remove those (autogenerated?) comments all over the PR.
They add a lot of unnecessary noise.
| * @param {string[]} lines - An array of strings, each string representing a line of the MRZ. | ||
| * @param {ParseMRZOptions} options - ParseMRZOptions | ||
| * @returns the result of the getResult function. | ||
| */ | ||
| export default function parseTD1(lines: string[], options: ParseMRZOptions) { |
There was a problem hiding this comment.
Could you add types in return values to all the parsers (this one but also TD2, TD3 etc...)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
added type definitions to the results for MRZ as well as consolidated all the types into a singular file and exported them from the index.ts file for ease of use with developers.
also added comments and documentation where possible and some mild lint clean up