-
-
Notifications
You must be signed in to change notification settings - Fork 34
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
[REFACTORING] : introduce const MAPPER_DEFAULT #18
Comments
Hi I would like to work this issue for you. Can I please have write access to this project to push my branch? Thank you |
Hi, I nice to hear that :) it is awesome, that some other peoples than me wants to contribute this project ❤️ I can not give you write access, however you have a possibility to fork repo by the button on up&right display corner Then this repo should be clone on your profiles, with fully write access for you. (probably hosted as cmwalshWVU/xlsx-import). There It is recommend to commit changes into new branch. Finally, when your fix is done and all commits pushed, please go to "Pull requests" and create new one targeting to this repo. @lowkeypriority @cmwalshWVU Thank you again for contributing this library :) If you want, I may create more well described issues here and on siemienik/xlsx-renderer labeled 'good first issue' . What do you think? |
@Siemienik Thank you.
It would be great if you could do this. This will make it easier to contribute and implement your ideas :) |
I've created issue with list of mappers to develop: #21 |
Actual
Current default mapper is defined implicitly in two places:
https://github.com/Siemienik/xlsx-import/blob/be72568ed68b1922c81731e45beecd89c3283ce9/src/Importer.ts#L22
https://github.com/Siemienik/xlsx-import/blob/be72568ed68b1922c81731e45beecd89c3283ce9/src/Importer.ts#L41
Expected
The default mapper should be defined explicitly as a const
MAPPER_DEFAULT
(or some name like that)The text was updated successfully, but these errors were encountered: