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

Always use double quotes for comments #177

Closed
wants to merge 1 commit into from
Closed

Conversation

peterdesmet
Copy link
Member

When trying to fix double quotes in a comment (5a35c10), I noticed that the CSV with comments for each term (https://github.com/tdwg/dwc/blob/use-quotes/build/config/terms.csv) only uses double quotes for the comment when necessary (e.g. when there was a comma in the comment).

I think this can lead to problems if the file is edited manually: introducing a comma in the comment field would break the csv. This pull requests adds quotes for all comments. It does not affect the guide and the CSV is now rendered correctly on GitHub: https://github.com/tdwg/dwc/blob/8fabcf056b9eb30fe08b8169bef25bdcbc3f40c9/build/config/terms.csv

Note 1: I wonder what will happen if this file is imported and exported in Google Refine though. Will quotes be stripped again?

Note 2: If this PR is not accepted, 5a35c10 still needs to be fixed in some way.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@tucotuco tucotuco mentioned this pull request Oct 20, 2018
@tucotuco
Copy link
Member

I don't think this is a good idea for the reasons given. Editing in anything that does a CSV export will not be able to reproduce this rule around a subset of the fields in the file. It COULD enclose all fields in double quotes though. No matter what, there is the possibility of messing things up when editing manually. I would suggest to leave it as it is.

@peterdesmet
Copy link
Member Author

Agreed to not implement/enforce

@tucotuco tucotuco deleted the use-quotes branch August 7, 2021 00:16
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.

None yet

2 participants