Conversation
qcdyx
changed the title
emmambd
requested review from
tzujenchanmbd
and removed request for
tzujenchanmbd
🚨 Code Formatting Issue 🚨This contribution does not follow the conventions set by the Google Java Style Guide. Please run the following command at the root of the project to fix formatting errors: ./gradlew spotlessApply🗂️ Affected files
Go to the Actions Dashboard to download the full Spotless output |
|
@qcdyx I'm confused here - I thought the plan was to add the |
📝 Acceptance Test Report📋 Summary✅ The rule acceptance has passed for commit acc0560 📊 Notices ComparisonNew Errors (1 out of 1801 datasets, ~0%) ✅Details of new errors due to code change, which is less than the provided threshold of 1%.
Dropped Errors (1 out of 1801 datasets, ~0%) ✅Details of new errors due to code change, which is less than the provided threshold of 1%.
New Warnings (0 out of 1801 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Warnings (0 out of 1801 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check0 out of 1801 sources (~0 %) are corrupted. ⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
📜 Memory Consumption
|
|
I understand the confusion. Initially, the plan was to have the duplicated_element trigger contribute to the malformed geojson notice. However, we discovered that with Gson, this approach isn’t viable. Gson follows standard JSON parsing rules, meaning that if there are duplicate keys in a JSON object, the last occurrence will simply overwrite any previous ones. It doesn’t throw an exception or flag an error for duplicated elements. Consequently, a duplicated element doesn’t trigger the MalformedJsonNotice as we might have expected. @emmambd |
|
@qcdyx Clear! Thanks for explaining this. |
🚨 Code Formatting Issue 🚨This contribution does not follow the conventions set by the Google Java Style Guide. Please run the following command at the root of the project to fix formatting errors: ./gradlew spotlessApply🗂️ Affected files
Go to the Actions Dashboard to download the full Spotless output |
| import org.mobilitydata.gtfsvalidator.annotation.GtfsValidationNotice; | ||
|
|
||
| /** Unknown elements in locations.geojson file. */ | ||
| @GtfsValidationNotice(severity = ERROR) |
| public GeoJsonUnknownElementNotice(String filename, String unknownElement, String message) { | ||
| this.filename = filename; | ||
| this.unknownElement = unknownElement; | ||
| this.message = message; |
There was a problem hiding this comment.
If we have the element, do we need the message?
There was a problem hiding this comment.
I added the message field to maintain consistency with GeoJsonDuplicatedElementNotice, but I can remove it if it's not necessary.
core/src/main/java/org/mobilitydata/gtfsvalidator/notice/GeoJsonDuplicatedElementNotice.java
Outdated
Show resolved
Hide resolved
📝 Acceptance Test Report📋 Summary✅ The rule acceptance has passed for commit ba3b425 📊 Notices ComparisonNew Errors (1 out of 1801 datasets, ~0%) ✅Details of new errors due to code change, which is less than the provided threshold of 1%.
Dropped Errors (1 out of 1801 datasets, ~0%) ✅Details of new errors due to code change, which is less than the provided threshold of 1%.
New Warnings (0 out of 1801 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Warnings (0 out of 1801 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check0 out of 1801 sources (~0 %) are corrupted. ⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
📜 Memory Consumption
|
main/src/main/java/org/mobilitydata/gtfsvalidator/table/GeoJsonFileLoader.java
Outdated
Show resolved
Hide resolved
main/src/main/java/org/mobilitydata/gtfsvalidator/util/geojson/MapJsonTypeAdapter.java
Outdated
Show resolved
Hide resolved
…ityData/gtfs-validator into 1930-flex-unknown_geo_json_element
🚨 Code Formatting Issue 🚨This contribution does not follow the conventions set by the Google Java Style Guide. Please run the following command at the root of the project to fix formatting errors: ./gradlew spotlessApply🗂️ Affected files
Go to the Actions Dashboard to download the full Spotless output |
📝 Acceptance Test Report📋 Summary✅ The rule acceptance has passed for commit 7dc771a 📊 Notices ComparisonNew Errors (0 out of 1809 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Errors (0 out of 1809 datasets, ~0%) ✅No changes were detected due to the code change. New Warnings (0 out of 1809 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Warnings (0 out of 1809 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check0 out of 1809 sources (~0 %) are corrupted. ⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
📜 Memory Consumption
|
qcdyx
changed the title
🚨 Code Formatting Issue 🚨This contribution does not follow the conventions set by the Google Java Style Guide. Please run the following command at the root of the project to fix formatting errors: ./gradlew spotlessApply🗂️ Affected files
Go to the Actions Dashboard to download the full Spotless output |
🚨 Code Formatting Issue 🚨This contribution does not follow the conventions set by the Google Java Style Guide. Please run the following command at the root of the project to fix formatting errors: ./gradlew spotlessApply🗂️ Affected files
Go to the Actions Dashboard to download the full Spotless output |
📝 Acceptance Test Report📋 Summary✅ The rule acceptance has passed for commit c549f04 📊 Notices ComparisonNew Errors (0 out of 1809 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Errors (0 out of 1809 datasets, ~0%) ✅No changes were detected due to the code change. New Warnings (0 out of 1809 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Warnings (0 out of 1809 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check0 out of 1809 sources (~0 %) are corrupted. ⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
📜 Memory Consumption
|
📝 Acceptance Test Report📋 Summary✅ The rule acceptance has passed for commit 71fc2b6 📊 Notices ComparisonNew Errors (0 out of 1809 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Errors (0 out of 1809 datasets, ~0%) ✅No changes were detected due to the code change. New Warnings (0 out of 1809 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Warnings (0 out of 1809 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check0 out of 1809 sources (~0 %) are corrupted. ⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
📜 Memory Consumption
|
📝 Acceptance Test Report📋 Summary✅ The rule acceptance has passed for commit 0aad674 📊 Notices ComparisonNew Errors (0 out of 1809 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Errors (0 out of 1809 datasets, ~0%) ✅No changes were detected due to the code change. New Warnings (0 out of 1809 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Warnings (0 out of 1809 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check0 out of 1809 sources (~0 %) are corrupted. ⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
📜 Memory Consumption
|
|
|
||
| /** Unknown elements in locations.geojson file. */ | ||
| @GtfsValidationNotice(severity = INFO) | ||
| public class GeoJsonUnknownElementNotice extends ValidationNotice { |
There was a problem hiding this comment.
Please remove this class if it won't be part of the implementation in this PR.
| @@ -0,0 +1,19 @@ | |||
| package org.mobilitydata.gtfsvalidator.util.geojson; | |||
|
|
|||
| public class UnknownJsonKeyException extends RuntimeException { | |||
There was a problem hiding this comment.
Please remove this class if it won't be part of the implementation in this PR.
📝 Acceptance Test Report📋 Summary✅ The rule acceptance has passed for commit d166eb9 📊 Notices ComparisonNew Errors (0 out of 1809 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Errors (0 out of 1809 datasets, ~0%) ✅No changes were detected due to the code change. New Warnings (0 out of 1809 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Warnings (0 out of 1809 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check0 out of 1809 sources (~0 %) are corrupted. ⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
📜 Memory Consumption
|
* add json map type adapter and catched JsonSyntaxException Co-authored-by: David Gamez Diaz <1192523+davidgamez@users.noreply.github.com>
3 participants
Summary:
Closes #1929
Expected behavior:
Please make sure these boxes are checked before submitting your pull request - thanks!
gradle testto make sure you didn't break anything