firegloves / MemPOI

A library to simplify export from database to Excel files using Apache POI :japanese_goblin:
MIT License
57 stars 7 forks source link

Feature/customize column header #23

Closed bdzzaid closed 3 years ago

bdzzaid commented 3 years ago

This feature is expected to manage I18n column names. Can you please review and let me know if any update is needed.

I hope you can quickly release it

bdzzaid commented 3 years ago

Hi @firegloves

I checked the failure on "Build and Test / Codacy Analysis CLI" step. It seems due to an authentication issue to codacy plateform.

Can you check from your side please ?

Thks Zaïd

firegloves commented 3 years ago

Hi @bdzzaid,

I added some comments in the review. However good job, thx for the contribution :) I have only to ask you to add some integration tests. In my opinion we need at least these tests:

If you can, please improve the unit test already written, passing also a null value to the builder. I know that's already managed, but it could help to prevent regressions.

For integrations tests, you can find a docker environment in the docker_data folder. It is already configured, just start it and copy-paste an already existing class: it will be using the docker DB. The AssertionHelper class is a bit tricky to use, I should refactor it a bit. You can find there some examples to validate the generated files, but if you encounter too many difficulties I can take care to complete the integration tests. But these ones are demanding days at work and I have no time to work on Mempoi, so if you need it as soon as possible, consider spending a bit of time on this task

Don't worry about the code analysis, I don't know right now how to propagate the missing credentials, but:

Thx again for your work!