-
Notifications
You must be signed in to change notification settings - Fork 101
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
feat: Add metadata to report #1389
Conversation
This contribution does not follow the conventions set by the Google Java style guide. Please run the following command line at the root of the project to fix formatting errors: |
✅ Rule acceptance tests passed. |
✅ Rule acceptance tests passed. |
✅ Rule acceptance tests passed. |
This contribution does not follow the conventions set by the Google Java style guide. Please run the following command line at the root of the project to fix formatting errors: |
@isabelle-dr This is still a work in progress, and obviously the report needs better organization, but wanted to show you the data as it's currently being output and see if you had any specific requests for organizing it on the report: |
@bdferris-v2 @davidgamez Leaving this a draft for now, but looking for any feedback/suggestions on the approach and/or the report organization/layout |
This contribution does not follow the conventions set by the Google Java style guide. Please run the following command line at the root of the project to fix formatting errors: |
As linked to the GitHub issue, here is a scoping doc for this PR. |
8e70ba5
to
277ef2e
Compare
This is beautiful (and why so many shapes in this dataset? 😅 is 1171 the number of unique shape_ids or the number of records?) |
✅ Rule acceptance tests passed. |
@isabelle-dr I've updated it to always count unique IDs and simplified the code a bit to use generics (@davidgamez open to feedback on this if there is a cleaner, more concise approach). |
✅ Rule acceptance tests passed. |
@briandonahue we're getting there! Another question I have is: under the "Files included", are you listing all of them or only the official GTFS files? |
main/src/main/java/org/mobilitydata/gtfsvalidator/report/model/FeedMetadata.java
Outdated
Show resolved
Hide resolved
main/src/main/java/org/mobilitydata/gtfsvalidator/runner/ValidationRunner.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Yes, these were 2 I was testing with:
I was not, but I've updated it now to do so. /cc @davidgamez |
✅ Rule acceptance tests passed. |
@briandonahue can you share a screenshot of your latest version for alhambra-ca-us.zip? |
✅ Rule acceptance tests passed. |
@isabelle-dr Here you go 😄 |
Perfect! Merging |
Summary:
Closes #1363.
Add a metadata / overview section to the HTML report.
Expected behavior:
Explain and/or show screenshots for how you expect the pull request to work in your testing (in case other devices exhibit different behavior).
Please make sure these boxes are checked before submitting your pull request - thanks!
gradle test
to make sure you didn't break anything