search

danielfone / as_csv

Instant CSV support for Rails
MIT License
19 stars 9 forks source link

make #headers public on CsvBuilder #14

Open dzfolio opened 5 months ago

dzfolio commented 5 months ago

we have a use case where we'd like to generate only the headers. in order to generate the headers correctly we do need to look at all rows and collect/uniq them, why is why I am adding it to csv_builder

please let me know feedback

dzfolio commented 5 months ago

Hi @dzfolio. Thanks for the contribution! I don't mind this feature, but I wonder if it would be simpler to just make the AsCSV::CSVBuilder#headers method public? I realise this isn't functionally equivalent, but then if you only want the headers you can call headers on the CSVBuilder object instead of to_csv. How does this sound?

sure sounds reasonable to me. should I break out tests for #headers or do you think it's covered enough from existing tests

danielfone commented 5 months ago

should I break out tests for #headers or

Good question. I think since it's now part of the public interface it's worth having a few basic tests against it. You can even just copy the relevant to_csv contexts/specs and change the assertions to check the headers. I don't mind if the spec is not DRY in this case.