Skip to content
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

Allow custom column separator for CSV parsing in uploads controller #1470

Merged
merged 2 commits into from
Nov 18, 2024

Conversation

acflint
Copy link
Contributor

@acflint acflint commented Nov 17, 2024

Fix for this issue: #1462

csv_valid? wasn't using the submitted col_sep argument, so semicolon separated CSVs were failing to be validated.

@zachgoll
Copy link
Collaborator

@acflint disregard my comment on the other PR that was closed. This looks good!

@acflint
Copy link
Contributor Author

acflint commented Nov 18, 2024

@zachgoll I can fix the failing specs, if you'd like. Let me know.

@zachgoll
Copy link
Collaborator

@acflint yep, that would be great. Looks like we just need to pass col_sep as params for each of those failing tests

@acflint
Copy link
Contributor Author

acflint commented Nov 18, 2024

@zachgoll done 👍🏻

@zachgoll zachgoll merged commit 9cc9f42 into maybe-finance:main Nov 18, 2024
5 checks passed
@acflint
Copy link
Contributor Author

acflint commented Nov 18, 2024

I know it's small, but after almost 10 years of being a dev, this is the first OSS contribution I've had merged. So that's pretty cool :) Thanks @zachgoll!

@zachgoll zachgoll mentioned this pull request Nov 18, 2024
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