Skip to content

feat: Make file headers available in CSV table containers#850

Merged
lionel-nj merged 1 commit intoMobilityData:masterfrom
aababilov:csv-header
Apr 16, 2021
Merged

feat: Make file headers available in CSV table containers#850
lionel-nj merged 1 commit intoMobilityData:masterfrom
aababilov:csv-header

Conversation

@aababilov
Copy link
Copy Markdown
Collaborator

@aababilov aababilov commented Apr 14, 2021 •
edited
Loading

Some validation rules may need to check whether a certain column is
present. E.g., this can allow them to skip further validation.

Unit tests are simplified and mocks are removed.

@aababilov aababilov requested review from barbeau and lionel-nj April 14, 2021 11:08
@aababilov
Copy link
Copy Markdown
Collaborator Author

aababilov commented Apr 14, 2021

Note that JavaDocs do not have the @return or @param section. This conforms to Google Java style that says that redundant sections are not required if the main comment already explains everything.

@barbeau
Copy link
Copy Markdown
Member

barbeau commented Apr 14, 2021

Column names are case-insensitive. Real feeds can capitalize them.

https://gtfs.org/reference/static says under "File Requirements:"

All field names are case-sensitive.

@MobilityData/transit-specs Thoughts on this?

@lionel-nj
Copy link
Copy Markdown
Contributor

lionel-nj commented Apr 14, 2021

Hi @aababilov Thanks for working on this. I will soon be reviewing :)

@aababilov
Copy link
Copy Markdown
Collaborator Author

aababilov commented Apr 14, 2021

Column names are case-insensitive. Real feeds can capitalize them.

https://gtfs.org/reference/static says under "File Requirements:"

All field names are case-sensitive.

@MobilityData/transit-specs Thoughts on this?

Fair point! Let's keep column names case-sensitive. We can revisit that topic later if needed.

I would like to avoid extra discussion for this commit, so let's keep the old behavior.

@aababilov
Copy link
Copy Markdown
Collaborator Author

aababilov commented Apr 14, 2021

Hi @aababilov Thanks for working on this. I will soon be reviewing :)

Merci beaucoup!

@aababilov
feat: Make file headers available in CSV table containers
8dee0c7 
Some validation rules may need to check whether a certain column is
present. E.g., this can allow them to skip further validation.

Unit tests are simplified and mocks are removed.
@lionel-nj lionel-nj mentioned this pull request Apr 15, 2021
Closed
7 tasks
@aababilov
Copy link
Copy Markdown
Collaborator Author

aababilov commented Apr 15, 2021 via email

lionel-nj
lionel-nj approved these changes Apr 16, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@lionel-nj lionel-nj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @aababilov LGTM

barbeau
barbeau approved these changes Apr 16, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@barbeau barbeau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @aababilov! 👍

@lionel-nj lionel-nj merged commit 7a934f9 into MobilityData:master Apr 16, 2021
@aababilov
Copy link
Copy Markdown
Collaborator Author

aababilov commented Apr 16, 2021

Thanks for review!

@aababilov aababilov deleted the csv-header branch April 20, 2021 02:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@barbeau barbeau barbeau approved these changes

@lionel-nj lionel-nj lionel-nj approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@aababilov @barbeau @lionel-nj