Skip to content

Issue #3 Changing CSV to NimbleCSV#15

Merged
zachdaniel merged 4 commits intoash-project:mainfrom
Jatanasio:main
Feb 23, 2026
Merged

Issue #3 Changing CSV to NimbleCSV#15
zachdaniel merged 4 commits intoash-project:mainfrom
Jatanasio:main

Conversation

@Jatanasio
Copy link
Copy Markdown
Contributor

@Jatanasio Jatanasio commented Feb 2, 2026

Contributor checklist

Leave anything that you believe does not apply unchecked.

  • [ x] I accept the AI Policy, or AI was not used in the creation of this PR.
  • [ x] Bug fixes include regression tests
  • Chores
  • Documentation changes
  • Features include unit/acceptance tests
  • [ x] Refactoring
  • [ x] Update dependencies

I went in and added the NimbleCSV dependency, then updated the operators in data_layer.ex to use NimbleCSV instead of CSV. The "lines" variable being passed through various functions/methods in the code were replaced with "iodata" because the NimbleCSV operator equivalent to CSV.encode is dump_to_iodata, and so it was easier to keep the two separate when making chnages, I can go back and revert it to "lines" if you would prefer it that way. I also changed build_parser.ex to create the parse compiler as requested so that it will run at compile time.

For testing, as I understood the issue description, you did not want any changes to the actual functionality of the parsing logic, just to refactor CSV to NimbleCSV in order to eliminate the exit messages and optimize the runtime. I wasn't sure if you wanted any additional test for NimbleCSV specifically, and so for testing/regression testing I first removed all CSV operators and ran "mix test" to ensure that all the tests had failed, then after adding the operators for NimbleCSV and adding compiler logic to build_parser.ex I ran it again to ensure it passed all the tests using the refactored logic, ensuring both that CSV was properly removed from the logic and that NimbleCSV was performing correctly.

If you would like me to add additional tests to it for anything in specific I can add those as well. Sorry in advance if I missed anything I was supposed to do, this is my first pull request and I hope I was able to follow the workflow properly.

Comment thread lib/ash_csv/data_layer.ex Outdated
Comment thread lib/ash_csv/data_layer/transformers/build_parser.ex
Comment thread lib/ash_csv/data_layer/transformers/build_parser.ex Outdated
Comment thread mix.exs Outdated
zachdaniel
zachdaniel previously approved these changes Feb 3, 2026
@zachdaniel
Copy link
Copy Markdown
Contributor

Oops, meant to request changes not approve. There are a few small things to address here, but overwell very well done, thank you!

@zachdaniel zachdaniel merged commit 34dbfac into ash-project:main Feb 23, 2026
6 checks passed
@zachdaniel
Copy link
Copy Markdown
Contributor

🚀 Thank you for your contribution! 🚀

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